Skip to content

Conversation

@wallak
Copy link

@wallak wallak commented Mar 15, 2017

This merge add the linux CONFIG_COMPAT layer (This allows a 32 birs crystalhd library to run on a 64 bits linux kernel), and some other fixes.

Copy link
Collaborator

@dlenski dlenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This merge add the linux CONFIG_COMPAT layer (This allows a 32 birs crystalhd library to run on a 64 bits linux kernel)

I'm curious… what's the motivation for this? 64-bit versions of the crystalhd library exist and work fine in my experience.

… and some other fixes.

It appears that a substantial portion of the changes here are to remove unused code paths (several unused variables and a couple unused functions).

This is helpful, but could you rebase these into a separate commit, so that these changes are clearly identifiable?

pNALU = pStart + Ctx->VidParams.StartCodeSz;
ulNalSize = Ctx->PESConvParams.m_lStartCodeDataSize;
// pNALU = pStart + Ctx->VidParams.StartCodeSz; // uint8_t *pNALU = NULL;
// ulNalSize = Ctx->PESConvParams.m_lStartCodeDataSize; // uint32_t ulNalSize = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these variables were totally unused… is that right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed these variables are defined and useless.

The 'compat' layer was very useful in the past when the x86_64 was not the reference. Today I still have a working i686 distribution with some packages not in-sync with their x86_64 counterparts. This is still useful for testing, the i686 packages will run perfectly on the x86_64 kernel. We have too the x32 abi (x86_64 instructions with 32 bits pointer), this layer allows the compatibility too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the branch master on my side; The changes are rebased and separated.

I let you check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants