[x264-devel] Re: Patch for Scenarist HD compatibility

[Ian Caulfield]

On 1/8/07, Loren Merritt <lorenm at u.washington.edu> wrote:
>
> On Sat, 6 Jan 2007, Ian Caulfield wrote:
>
> > - fix a small bug in bitstream_write (on x86 a right shift of more than
> 32
> > bits actually shifts by the shift amount mod 32, rather than returning
> 0)
> >
> > --- common/bs.h (revision 616)
> > +++ common/bs.h (working copy)
> > @@ -185,7 +185,10 @@
> >  }
> >  else
> >  {
> > -     *s->p = (*s->p << s->i_left) | (i_bits >> (i_count - s->i_left));
> > +     if( (i_count - s->i_left) >= 32 )
> > +         *s->p = (*s->p << s->i_left);
> > +     else
> > +         *s->p = (*s->p << s->i_left) | (i_bits >> (i_count -
> s->i_left));
> >       i_count -= s->i_left;
> >       s->p++;
> >       s->i_left = 8;
>
> Not a bug. i_count is the number of bits passed in i_bits, which is a
> uint32_t, thus i_count<=32. i_left>0 because whenever it is 0 we increment
> the bitstream pointer and refill it. So the shift can't exceed 31.


Sorry, I should have explained further - I had the problem when bs_write_ue
was being called with a value having 23 significant bits - bs_write was then
being called with a bit count of 45. I assumed that the intended behaviour
was to zero-extend the value when the count exceeded 31. I could modify the
patch to make bs_write_ue make two calls to bs_write if you'd prefer.


> bs_align_10 doesn't look right. shouldn't you use bs_rbsp_trailing
> instead?


The difference is the one bit is optional - if the end of the SEI payload is
byte aligned, nothing further is written. From the (draft) spec:

sei_payload( payloadType, payloadSize )
{
    <snip payload contents>
    if( !byte_aligned() )
    bit_equal_to_one
    while ( !byte_aligned() )
        bit_equal_to_zero
}


> - move the version info SEI header after the SPS and PPS headers
> > (required for Scenarist to identify the stream as an H264 ES)
>
> Bugs should be fixed where the bug is. This is a bug in Scenarist, so out
> of principle I won't work around it in x264.


Could it be a requirement of the HD DVD spec that H264 streams start with
AUD, SPS, PPS? I don't have access to that spec...

I did consider adding a flag to remove the version header from the
bitstream, but when I found that reordering was sufficient I thought that
just patching that would be fairly unobtrusive

On Sun, 7 Jan 2007, bond wrote:

> or HDDVD compliance is wanted...
>

> The whole patch is independent of whether HDDVD compliance is wanted. It
> just adds a feature that Scenarist requires, because Scenarist is too
> lazy to implement its own VBV model. (hint: a smart muxer does _not_
> need to know what VBV parameters the stream was encoded with, it only
> needs to know what VBV parameters the decoder wants)


Interestingly enough, Scenarist doesn't require the presence of VBV
parameters in VC-1 streams

Ian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.videolan.org/pipermail/x264-devel/attachments/20070108/a2f1771c/attachment.htm