[FFmpeg-devel] [PATCH] mov auto crop

[Aurelien Jacobs]

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.

> Feel free to add another field in AVFormatContext if you want, but I
> won't do it.

Like in attached patch ?

> Btw, when are we going to have cover art support in svn ? I mean I've
> had a patch using AVMetadata since January.

Hum... I think it's in svn since quite a long time now.
Try this sample for example:
  http://samples.mplayerhq.hu/Matroska/ticket-a_aac.mkv

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: crop_api.diff
Type: text/x-diff
Size: 600 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101003/0528a563/attachment.diff>