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

[Nicolas George]

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.

> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190516/46bea257/attachment.sig>