On Thu, May 16, 2019 at 12:04 PM Paul B Mahol <one...@gmail.com> wrote:
> On 5/16/19, Nicolas George <geo...@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".