On 5/20/2017 1:55 PM, James Almer wrote:
> On 5/11/2017 9:56 AM, Michael Niedermayer wrote:
>> On Wed, May 10, 2017 at 11:06:52PM -0300, James Almer wrote:
>>> On 5/10/2017 9:55 PM, Michael Niedermayer wrote:
>>>> On Wed, May 10, 2017 at 10:41:30AM -0300, James Almer wrote:
>>>>> On 5/9/2017 11:56 PM, Michael Niedermayer wrote:
>>>>>> On Mon, May 08, 2017 at 03:46:23PM -0300, James Almer wrote:
>>>>>>> From: Anton Khirnov <an...@khirnov.net>
>>>>>>>
>>>>>>> The current condition can trigger in cases where it shouldn't, with
>>>>>>> unexpected results.
>>>>>>> Make sure that:
>>>>>>> - container cropping is really based on the original dimensions from the
>>>>>>> caller
>>>>>>> - those dimenions are discarded on size change
>>>>>>>
>>>>>>> The code is still quite hacky and eventually should be deprecated and
>>>>>>> removed, with the decision about which cropping is used delegated to the
>>>>>>> caller.
>>>>>>> ---
>>>>>>> This merges commit 4fded0480f20f4d7ca5e776a85574de34dfead14 from libav
>>>>>>>
>>>>>>> libavcodec/h264_slice.c | 20 +++++++++++++-------
>>>>>>> libavcodec/h264dec.c | 3 +++
>>>>>>> libavcodec/h264dec.h | 5 +++++
>>>>>>> 3 files changed, 21 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>>>>>>> index acf6a73f60..a7916e09ce 100644
>>>>>>> --- a/libavcodec/h264_slice.c
>>>>>>> +++ b/libavcodec/h264_slice.c
>>>>>>> @@ -378,6 +378,8 @@ int ff_h264_update_thread_context(AVCodecContext
>>>>>>> *dst,
>>>>>>> h->avctx->coded_width = h1->avctx->coded_width;
>>>>>>> h->avctx->width = h1->avctx->width;
>>>>>>> h->avctx->height = h1->avctx->height;
>>>>>>> + h->width_from_caller = h1->width_from_caller;
>>>>>>> + h->height_from_caller = h1->height_from_caller;
>>>>>>> h->coded_picture_number = h1->coded_picture_number;
>>>>>>> h->first_field = h1->first_field;
>>>>>>> h->picture_structure = h1->picture_structure;
>>>>>>
>>>>>>> @@ -874,13 +876,17 @@ static int init_dimensions(H264Context *h)
>>>>>>> av_assert0(sps->crop_top + sps->crop_bottom < (unsigned)h->height);
>>>>>>>
>>>>>>> /* handle container cropping */
>>>>>>> - if (FFALIGN(h->avctx->width, 16) == FFALIGN(width, 16) &&
>>>>>>> - FFALIGN(h->avctx->height, 16) == FFALIGN(height, 16) &&
>>>>>>> - h->avctx->width <= width &&
>>>>>>> - h->avctx->height <= height
>>>>>>> - ) {
>>>>>>> - width = h->avctx->width;
>>>>>>> - height = h->avctx->height;
>>>>>>> + if (h->width_from_caller > 0 && h->height_from_caller > 0 &&
>>>>>>> + !sps->crop_top && !sps->crop_left &&
>>>>>>> + FFALIGN(h->width_from_caller, 16) == FFALIGN(width, 16) &&
>>>>>>> + FFALIGN(h->height_from_caller, 16) == FFALIGN(height, 16) &&
>>>>>>> + h->width_from_caller <= width &&
>>>>>>> + h->height_from_caller <= height) {
>>>>>>> + width = h->width_from_caller;
>>>>>>> + height = h->height_from_caller;
>>>>>>> + } else {
>>>>>>> + h->width_from_caller = 0;
>>>>>>> + h->height_from_caller = 0;
>>>>>>> }
>>>>>>
>>>>>> With this, seeking in a file could affect if croping is used
>>>>>> would something break if croping was unaffected by what was priorly
>>>>>> decoded ?
>>>>>
>>>>> I don't know. Do you have a test case where this could break that i can
>>>>> check?
>>>>
>>>> no, it was just an thought that came to my mind when looking at the
>>>> code. I dont remember seeing this break anything, it changed some
>>>> files output though unless these were caused by another patch i had
>>>> locally
>>>
>>> Could you try to confirm they weren't changed by this patch? Unless i'm
>>> reading it wrong, this set afaik isn't supposed to change the output of
>>> the decoder (at least not negatively), as reflected by fate, just move
>>> the cropping logic to decode.c
>>
>> I retested, it was
>> [3/4] h264dec: export cropping information instead of handling it internally
>> that caused the changes
>> changes seen are with CVFC1_Sony_C.jsv and tickets/4274/sample.ts
>>
>> 4247 needs "-threads 1 -flags2 showall -ss 7" for showin the
>> difference, the sony file shows a difference with just plain default
>> reencoding to avi
>>
>> Our fate test doesnt change, i guess due to -flags unaligned in it
>>
>> thx
>
> CVFC1_Sony_C.jsv is fixed now that the cropping logic works correctly.
> tickets/4274/sample.ts still shows the difference, but it changes
> garbage output with slightly different, less ugly but still garbage output.
>
> Old: http://0x0.st/ghF.png
> New: http://0x0.st/ghC.png
>
> Unless this can be reproduced with negative effects with a sane file and
> not with a badly cut mpegts stream with missing references that requires
> seeking and a some specific flags, i'm inclined to not consider it worth
> bothering with.
>
> I'll be pushing the set sometime next week if no other regressions are
> found.
Pushed, thanks.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
