[FFmpeg-devel] [PATCH] CrystalHD decoder support v6

[Michael Niedermayer]

On Sun, Mar 06, 2011 at 10:26:11AM -0800, Philip Langdale wrote:
> On Sun, 6 Mar 2011 04:40:36 +0100
> Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > see libavcodec/h264_parser.c:
> >             if(h->sps.frame_mbs_only_flag){
> >                 h->s.picture_structure= PICT_FRAME;
> >             }else{
> >                 if(get_bits1(&h->s.gb)) { //field_pic_flag
> >                     h->s.picture_structure= PICT_TOP_FIELD +
> > get_bits1(&h->s.gb); //bottom_field_flag } else {
> >                     h->s.picture_structure= PICT_FRAME;
> >                 }
> >             }
> > 
> > above will tell you if its top/bottom field or frame
> > iam not completely sure how to cleanly hook that up though but its
> > likely better than hardcoding guesses.
> > mpeg2 field pictures can also be detected through the corresponding
> > parser
> 
> Yeah, but doing that will require introducing a lot more knowledge of
> the bitstream, which I desperately want to avoid - hence why I'm hoping
> I can get firmware fixes out of Broadcom so that it reports picture
> characteristics correctly.

why not just open the parser for the current codec and parse the frame?


>  
> > 
> > 
> > > +    /*
> > > +     * XXX: Is the first field always the first one encountered?
> > > +     */
> > 
> > this question is not very well written "is the X always the X"
> 
> Indeed. It's meant to ask "Is the top field always the first one
> encountered?" - ie: Are fields always ordered top -> bottom in the
> bistream even if the display order is bottom -> top?

no


> 
> > 
> > reget_buffer() doesnt feel correct here, why do you need it instead of
> > get_buffer()?
> 
> It helps reduce the amount of state I have to maintain when dealing with
> interlaced frames. Without it, I'd have to keep the previous buffer if
> I only got one frame on the first decode() call so that I could write
> the second field in on the next call. with reget_buffer(), I can just
> write the field without caring.

reget_buffer() is expensive (memcpy of whole frame) in many cases


> 
> > also might be able to directly return the frames from the decoder
> > without using get_buffer()
> 
> So, the driver and library design mean that there's no way to get the
> hardware to write directly to the final buffer - so there are always
> going to be more copies than you really want. In the specific case here,
> I believe there's always going to be a copy - it's just a question of
> who does it. You can ask the library to do it, but I never got that to
> work, it would always muck up the stride. Or you could try and pass the
> library buffer out, but in a real client like mplayer, that just means
> the client will immediately do a copy after the decode() because it
> can't guarantee how long the buffer is available for, and if you're
> doing accelerated rendering, it has to be in an acceleratable buffer
> anyway. So (re)get_buffer is obtaining an acceleratable buffer and we
> end up in the same place. Finally, interlaced content means you need
> the separate buffer to merge the fields.

reget_buffer() unlikely to return a buffer in video ram
because we have to copy it on the next reget_buffer() call and that means
read and read from video ram is slow
The reason for the copy is that the decoder can need it again before it
has been displayed 

also there can be a filter afterwards (deinterlace, deblock, ...)
in that case outputting the frame as is means 1 copy less


[...]
> So, if I move this to another thread, the functional difference would be
> that during the waiting period, decode() would return immediately and
> not consume any input bytes or return any frames, rather than blocking
> inside decode(). Does that make any difference in practice?
> 
> As for why it needs to wait: If you don't wait and just keep feeding
> packets in, you will build up a huge pipeline (100+ packets) and the
> client is likely to freak out in some way. As you know, mplayer
> maintains its own pts queue and after 24 pts are queued, it just starts
> dropping them and sync goes out the window.

a fix of the waiting will need changes to the client too i guess



[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110308/61aaf0cf/attachment.pgp>