Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exponential golomb encoding fixes #20

Closed
spiderkeys opened this issue Apr 20, 2017 · 3 comments
Closed

Exponential golomb encoding fixes #20

spiderkeys opened this issue Apr 20, 2017 · 3 comments

Comments

@spiderkeys
Copy link

I don't know if it will be helpful, but I made a bunch of changes to this library to fix some problems that I was seeing in my h264 stream. In particular, there were instances where some SPS/PPS golomb encoded values were not encoding correctly due to missing support for "big" values. H264 allows for values in some fields which are larger than what your bs_write_ue method allowed. The folks that work on the x264 library had to fix this some time back as well, so I formulated my solution based on their golomb encode/decode source. See the "big" variant that I added.

There are other various changes added as well, though mostly for my own purposes, so they may not be useful (or may be wrong in accordance with your intentions). Just thought I'd throw my work up here in case it was useful to anyone else:

https://gitlab.com/openrov/h264bitstream/tree/openrov

@aizvorski
Copy link
Owner

aizvorski commented Apr 20, 2017

@spiderkeys I'm glad you're using (and modifying) h264bitstream! Let's see if we can get to the bottom of this issue, I'd like to merge your changes if possible.

In the current master, it looks like bs_write_ue can handle any size up to 32 bit, due to the following code that calculates lengths from the 8-bit table:

        if (v >= 0x01000000)
        {
            len = 24 + len_table[ v >> 24 ];
        }
        else if(v >= 0x00010000)
        {
            len = 16 + len_table[ v >> 16 ];
        }
        else if(v >= 0x00000100)
        {
            len =  8 + len_table[ v >>  8 ];
        }

However, your bs_write_ue (not the _big version) only has this:

    bs_write_u( b, x264_ue_size_tab[ val + 1 ], val + 1 );

which would be expected to fail for any val > 0x100. Also x264_ue_size_tab is now called len_table.

I think this has been in master since ebf37e8.

Could you let me know which version of the library your fork is based on?

@aizvorski
Copy link
Owner

This appears to be fixed since 0.1.6. Please reopen if you still see this problem in the latest version. Thanks!

@spiderkeys
Copy link
Author

Sorry, I missed your previous comment!

I compared your latest version to my modified version and they produced identical SPS and PPS results, so it looks like everything is good!

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

No branches or pull requests

2 participants