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

[Jun Li]

On Thu, May 16, 2019 at 12:04 PM Paul B Mahol <onemda at gmail.com> wrote:

> 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.
>

Thanks Nicolas for clarifying the overhead of a filter.
That is the only reason why I choose the other path, avoid filter for
non-rotate/flip frames.
I will try to implement it as suggested.


Would it modify size of output frame by any chance?


Hi Paul,
I believe so, like from landscape to portrait when rotation is applied.
The frame might be stretched in this case if we make a video from sequence
of images. Codec "copy" may still work since it should not falls into this
code path.
But correct me if I am wrong. I didnot test it. Thanks.

Best Regards,
-Jun

>
> >> > 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
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".