[FFmpeg-devel] [PATCH v3 2/2] fftools/ffmpeg: add support for per frame rotation and flip

Paul B Mahol onemda at gmail.com
Thu May 16 21:56:52 EEST 2019


On 5/16/19, Nicolas George <george at nsup.org> wrote:
> Jun Li (12019-05-16):
>> Sure.
>> This patch is checking the frame's orientation status, and apply input
>> filter if necessary.
>>    frame 1 -> check orientation -> get 2  -> need flip --> goto filter
>>    frame 2 -> check orientation -> get 1 -> do nothing
>>    frame 3 -> check orientation -> get 7 -> need flip -> goto filter
>>
>> The following is my understanding of using a filter to achieve, please
>> correct me if I am wrong.
>>    frame 1 -> goto filter -> check orientation -> get 2 -> flip
>>    frame 2 -> goto filter -> check orientation -> get 1 -> do nothing
>>    frame 3 -> goto filter -> check orientation -> get 7 -> flip
>>
>> This means the filter will always active no mater what type of content it
>> is (because we cannot get orientation until decode the frame).
>> The above is my understanding, correct me if I am wrong.
>
> Yes, that is exactly what I mean: update the transpose filter so that it
> has a mode where it applies the rotation from metadata automatically.
> The overhead for a single filter is rather small; we have a few cases
> where a filter is inserted to do nothing, just to be a placeholder.

Would it modify size of output frame by any chance?

>
>> > The name of this function suggests it is just a test, while it seems to
>> > do more.
>>
>> I see your point and agree with it. Either split the function or rename
>> it,
>> I prefer rename but still cannot think of a good name.
>> Let me re-think about it and will address maybe next iteration. I would
>> appreciate it if you could share some advice. :)
>
> This is entirely your choice, depending on how you want to organize your
> code. But if you make it the work of the filter, you do not need that
> code at all.
>
>> > All this code seems quite redundant with the part in ffplay.
>>
>> True, I see the code was like that before this. But is the merge work
>> should be included in this task or address in different one ?
>
> My opinion is that if a patch makes a duplicated piece of code more
> complex, then it should de-duplicate it beforehand.
>
>> I verified the VLC, they do the flip during playing, exactly as you
>> suggested.
>> The reason I didn't do that is, to keep the code change small to address
>> the ticket only.
>> Surely will address it if you think it's necessary.
>
> In the end, you decide what you want to do.
>
> Regards,
>
> --
>   Nicolas George
>


More information about the ffmpeg-devel mailing list