Exponential golomb encoding fixes

spiderkeys commented 7 years ago

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 commented 7 years ago

@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 ebf37e8cac8054c3fd55a8175472408ad602b88e.

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

aizvorski commented 6 years ago

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

spiderkeys commented 6 years ago

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!

aizvorski / h264bitstream

Exponential golomb encoding fixes #20