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

[Philip Langdale]

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.
 
> 
> 
> > +    /*
> > +     * 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?

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

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

> 
> would it make sense to move all this to a seperate thread to avoid
> blocking the outside vissible decoder with usleep() calls? or is the
> waiting time negliible?

The wait is definitely not negligible - it's measured in 10s of
milliseconds. The big challenge is that it's almost impossible to
know when the right behaviour is to wait and when you need to feed
another packet into the hardware. The hardware won't tell you it needs
more packets to construct the next frame, and even counting packets
up to h.264 maximums won't help because of mpeg ps/ts where packets !=
pictures. And sure, I could drag in a bunch of parsers to try and work
it all out but that will complicate things immensely.

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.

--phil
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110306/b4d9e578/attachment.pgp>