So, following up from the discussion on IRC, I think it better to start out
fresh.

I think I have fixed cropping for the right and bottom offset (with just
minor modifications and only two files touched).
What this patch does is:
1. keep the video size (width and height) to the full size of the video
surface (in ff_h264_decode_seq_parameter_set() );
2. apply the cropping on the picture when the whole frame has been decoded
(near the return value of decode_frame);
3. apply the cropping one more time on the context when the decoder is
closed (in h264_decode_end() );
4. remove the error for the "brainfart cropping".

Wrt 1, I'm aware that coded_width and coded_height should be used to
represent the full bitstream dimensions, but they are not present in
H264Context; the ones in the AVCodecContext interact in unclear ways (to me
at least) with width and height (see avcodec_set_dimensions);
Wrt 2, this is fine and it's what the h264 specs actually mandate;
Wrt 3, I had to add this because otherwise try_decode_frame would report
incorrect dimensions and subsequent frame decoding would fail.
(wrt 4, I never understood why it was 'brainfart'... theoretically you
could crop any offset in any direction, but oh well)

I think that other approaches might require a LOT more modifications and
readability would be hindered (imho).
Also, I would like to defend the design choice of applying the cropping at
the end of frame decoding on the picture itself (even if avframe values
become different) because this exactly what cropping is for: decode the
whole frame and display just a portion of it, ignoring anything outside it.

Finally, this patch fixes half of bug 378 (and does not break FATE).

Cheers,
Vittorio

On Thu, Feb 7, 2013 at 6:53 PM, Vittorio Giovara <[email protected]
> wrote:

> Yes, I ran 'make fate' on the FATE suite and no errors reported.
> Vittorio
>
>
> On Thu, Feb 7, 2013 at 4:59 PM, Luca Barbato <[email protected]> wrote:
>
>> On 07/02/13 16:08, Vittorio Giovara wrote:
>> > Thanks for the tip, I think i got it working.
>> > I cannot modify the values ff_get_buffer() as I go out of a H264Context
>> and
>> > lose the sps information, so I applied the cropping directly on the
>> AVFrame
>> > at the end of decode_frame(). This should be syntactically correct
>> > according to the spec anyways.
>> >
>> > Please see if the patches are fine for inclusion in the codebase.
>> > Cheers,
>> > Vittorio
>>
>> make fate works? I think it should solve at least one bug in bugzilla.
>>
>> lu
>>
>> _______________________________________________
>> libav-devel mailing list
>> [email protected]
>> https://lists.libav.org/mailman/listinfo/libav-devel
>>
>
>
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to