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

[bond]

any news on this? does it work right? any open bugs needed to be fixed? :)

  ----- Original Message ----- 
  From: Ian Caulfield 
  To: x264-devel at videolan.org 
  Sent: Monday, January 08, 2007 10:01 AM
  Subject: [x264-devel] Re: Patch for Scenarist HD compatibility


  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







------------------------------------------------------------------------------


  Internal Virus Database is out-of-date.
  Checked by AVG Free Edition.
  Version: 7.5.432 / Virus Database: 268.16.3/614 - Release Date: 2.1.2007 14:58
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.videolan.org/pipermail/x264-devel/attachments/20070113/c975fcc0/attachment.htm