So about a week has passed without comments or objections. Can this be applied then?
Thanks, Vittorio On Sat, Feb 9, 2013 at 2:11 AM, Vittorio Giovara <[email protected] > wrote: > 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 >>> >> >> >
0001-h264-correctly-interpret-right-cropping-offset-from-.patch
Description: Binary data
0002-h264-correctly-interpret-bottom-cropping-offset-from.patch
Description: Binary data
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
