Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop

From: Zhao Zhili <quinkblack@foxmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: "Tomas Härdin" <git@haerdin.se>
Subject: Re: [FFmpeg-devel] [PATCH v3 3/7] avcodec/mediacodecenc: use bsf to handle crop
Date: Thu, 15 Dec 2022 01:37:31 +0800
Message-ID: <tencent_F9B25B9234758048341F3ECD57539D4EE009@qq.com> (raw)
In-Reply-To: <724b8983e046dd29a2ebe9e64e8a00e165169f8c.camel@haerdin.se>

On Wed, 2022-12-14 at 18:08 +0100, Tomas Härdin wrote:
> tis 2022-12-13 klockan 11:20 +0800 skrev zhilizhao(赵志立):
> > 
> > > On Dec 12, 2022, at 23:27, Tomas Härdin <git@haerdin.se> wrote:
> > > 
> > > lör 2022-12-10 klockan 01:22 +0800 skrev Zhao Zhili:
> > > > From: Zhao Zhili <zhilizhao@tencent.com>
> > > > 
> > > > It's well known that mediacodec encoder requires 16x16
> > > > alignment.
> > > > Use our bsf to fix the crop info.
> > > > ---
> > > > v3: don't change the dimension for AV_PIX_FMT_MEDIACODEC. It
> > > > can
> > > > have
> > > > side effect.
> > > 
> > > Looks like this silently crops? Is that really a good idea? We
> > > usually
> > > don't do stuff like that. For example codecs that require even
> > > dimensions complain loudly then fail.
> > 
> > It’s reasonable to require even dimensions. Require dimensions
> > aligned
> > to 16 is uncommon. Everyone will complain why 1080x1920 doesn’t
> > work.
> > 
> > A lot of apps just use aligned dimensions. Users have no control on
> > these apps. It’s not the same with FFmpeg, users (developer or not)
> > can specify the dimension directly.
> 
> Wait a sec, I think I was misunderstanding what the code is doing.
> FFALIGN rounds *up*. Does this mean you insert fake data in the
> border
> that then gets cropped away, meaning the original essence is still
> "there"? That's a different thing and probably perfectly OK.

Yes, the dimension passed to MC is rounding up, then use bsf to remove
that border. It depends on our AVFrame data has been aligned properly.
This job is supposed to be done by any decent encoder, not by a
wrapper.

> 
> I think we might want something for this inside lavf somewhere, so
> that
> encoders can signal dimension alignment requirements. Some containers
> (MXF, MOV) support such cropping in a codec-agnostic manner.

From my own experience, dimension mismatch between codec and container
makes a lot of trouble. ISO base format specification specified how to
crop/scale after decoding clear, however, I don't think it has been
widely supported, including FFmpeg. We can fix that inside of FFmpeg,
but we should avoid such cases as much as we can.

> 
> > If we don’t fix it, either:
> > 
> > 1. Reject and fail directly. User complain why.
> > 2. Accept and keep going. Sometimes it works, sometimes don’t. It
> > depends on the device and get into a confused situation.
> > 
> > I know there are getWidthAlignment()/getHeightAlignment() to get
> > alignment info of codecs. The results are unreliable. The only
> > reliable way I can find is don’t depends on those API and fix it
> > by ourself.
> 
> Given how temperamental MC seems to be a "belt and braces" approach
> might be appropriate when dealing with it. Tell users (ffmpeg.c is a
> user here) that dimensions must be aligned by 16x16 and then
> automagically doing the required padding and cropping somewhere (lavf
> or ffmpeg.c) feels like a decent solution.
> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".