[FFmpeg-devel] [PATCH] mov auto crop

[Michael Niedermayer]

On Sun, Oct 03, 2010 at 12:46:59AM +0200, Aurelien Jacobs wrote:
> On Sat, Oct 02, 2010 at 01:01:43PM -0700, Baptiste Coudurier wrote:
> > On 10/2/10 10:15 AM, Aurelien Jacobs wrote:
> > > On Sat, Oct 02, 2010 at 02:57:43PM +0200, Benjamin Larsson wrote:
> > >> On 10/02/2010 04:51 AM, Baptiste Coudurier wrote:
> > >>> Hi guys,
> > >>>
> > >>> $subject.
> > >>>
> > >>> Please keep in mind that this issue has been present for _years_ and
> > >>> nobody dared to fix it. I believe the patch is not so hackish/ugly.
> > >>>
> > >>> I restrict it to codecs not supporting arbitrary resolutions that can be
> > >>> stored in .mov by quicktime.
> > >>>
> > >>> Maybe exporting to "crop", "wxh" is better, I don't know.
> > >>>
> > >>> I plan to add auto-rotate for iphone 3gs files this way as well :)
> > >>>
> > >>
> > >> Jolly good. But would it be possible to move this code to the api and
> > >> just not in the ffmpeg commandline tool?
> > > 
> > > I agree. Before this patch is committed, I would like to see a proper
> > > API patch, whether it is by documenting the metadata API or by adding
> > > some fields to the AVFormatContext struct.
> > > After we have a clean and documented API for this, it will be trivial
> > > and un-controversial to use this API in any muxer/demuxer and tools
> > > including all the API users which are not part of the FFmpeg repository.
> > > 
> > > I have to say that I don't like much the use of the metadata API for
> > > this. I forces every existing applications to filter out metadata before
> > > copying them to a transcoded file (which might have been resized, so the
> > > cropping values don't make any sens anymore). So in some way, it kind of
> > > break current public API...
> > 
> > You are being unreasonable IMHO. No it doesn't break the current public
> > API.
> 
> Unreasonable ? I just gave my feeling about this patch, nothing
> more. You can ignore it if you want.
> 
> > I already said that blindly copying metadata from one container to
> > another is a _bad_ idea and we should not do it.
> 
> We have some conversion tables specifically designed for this, and it
> sounds like a very common use case to re-encode for example from mp3 to
> vorbis while keeping metadata. In fact, I guess most users want to retain
> metadata while transcoding most files.
> Still it's not done blindly. You have to use the -map_meta_data option
> to trigger this.
> And users won't expect -map_meta_data to generate some broken file when
> transcoding, let's say from mov to mov, while resising video.

I think we need to pass the crop parameters through libavfilter so a
resize or crop filter can update them. Or a future uncrop filter could remove
them. (thats of course not for this patch to do but a future one but it
will solve the copying issues ...)

Also there are crop parameters in h264 that we dont support yet IIRC and
we might want these to use the same API. Id like to think a bit more about
this but maybe having the 2 crop parameters in AVCodecContext would work
better than AVStream. that said metadata with metadata per AVCodecContext
would work too


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101003/2508997e/attachment.pgp>