[FFmpeg-devel] [RCF] lavfi aspect ratio setting path (original) (raw)
Baptiste Coudurier baptiste.coudurier
Mon Jan 17 03:06:06 CET 2011
- Previous message: [FFmpeg-devel] [RCF] lavfi aspect ratio setting path
- Next message: [FFmpeg-devel] [RCF] lavfi aspect ratio setting path
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 1/16/11 6:01 PM, Michael Niedermayer wrote:
On Mon, Jan 17, 2011 at 02:52:29AM +0100, Michael Niedermayer wrote:
On Sun, Jan 16, 2011 at 04:16:01PM -0800, Baptiste Coudurier wrote:
On 1/16/11 4:00 PM, Michael Niedermayer wrote:
On Sun, Jan 16, 2011 at 03:23:23PM -0800, Baptiste Coudurier wrote:
On 1/16/11 3:20 PM, Michael Niedermayer wrote:
On Sun, Jan 16, 2011 at 04:56:42PM +0100, Stefano Sabatini wrote:
On date Sunday 2010-12-12 12:34:10 -0800, Baptiste Coudurier encoded:
On 12/3/10 9:53 PM, Baptiste Coudurier wrote:
On 12/2/10 6:28 PM, Michael Niedermayer wrote:
On Mon, Nov 29, 2010 at 03:26:40AM -0800, Baptiste Coudurier wrote: [...]
@@ -419,6 +430,10 @@
codec->width = ist->outputvideofilter->inputs[0]->w; codec->height = ist->outputvideofilter->inputs[0]->h; + ost->st->sampleaspectratio = codec->sampleaspectratio = + frameaspectratio == 0 ? // overriden by the -aspect cli option + avd2q(frameaspectratio*codec->height/codec->width, 255) : + ist->outputvideofilter->inputs[0]->sampleaspectratio; that looks odd if frameaspectratio == 0 then avd2q(frameaspectratio*codec->height/codec->width, 255) will be used but thats avd2q(0*codec->height/codec->width, 255)=0 Yes you are right, it required more modifications to make it work. Updated patch.
Ping. Any news on this? This is a show-stopper for many users. this patch does so many unrelated things iam just confused by it, i know some of the changes are definitly wrong and introduce bugs then for some i have no idea at all what they are supposed to do Like what ? the vfscale change after it there is no code left touching aspect in it but if you scale 320x240 to 320x320 then the sample aspect ratio changes. Well, currently -s does not change it, why -vf scale should change it ? because if the aspect is correct before scale it wont be correct anymore afterwards this happens for example in a real 640x480 ->320x200 rescaling There are changes in ffmpeg to code only excuted for stream copy (aka no filters, no scaling) that cant be correct, the stream copy handling should not change. You cannot set frameaspectratio unconditionally anymore if you want "-aspect" to work with libavfilter. And -aspect is required to work for the stream copy case., which makes the cli very inconsistent if "-aspect" does not work when encoding. I double checked my tree and I don't have these changes, this must be an old patch. We agreed that sampleaspectratio must be added to AVFilterLink, then according to this, ffmpeg.c is modified. Theres nothing wrong with adding it to AVFilterLink. But this whole thread IIRC started due to the command line option -aspect not working. This can be fixed by inserting a filter that sets the aspect instead of a second codepath to mess with the aspect. (we missed this option it seems or maybe the suggested solution looked simpler i dont remember...) Not only, aspect ratio information is not printed correctly in dumpformat and this heavily confuses people. And this patch looks like it attempts to do both at once. which i dont think is a good idea. I don't care about this, I want the situation fixed. It's been two Well as is i cannot review this patch because quite a few hunks make no sense and i dont even know which of the now 5+ things this patch combines they are related to. [...] * unrelated simplification of vftranspose that might or might not break it breaks, as aspect is defined like this in avcodec and coped from and to it over avfilter /** * sample aspect ratio (0 if unknown) * That is the width of a pixel divided by the height of the pixel. * Numerator and denominator must be relatively prime and smaller than 256 for some video standards. * - encoding: Set by user. * - decoding: Set by libavcodec. */ AVRational sampleaspectratio; the change turns 0 into inf not saying this definition is smart, IMHO it should be 0/0 for undefined but thats not how the current ABI/API defines it
What are you talking about ? I remove the picref->pixel_aspect field. The corresponding change must be to use AVFilterLink->sample_aspect_ratio. This hunk is correct.
The initialization of the source filter use 1:1 if it's undefined, "undefined" doesn't fit in IMHO.
-- Baptiste COUDURIER Key fingerprint 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA FFmpeg maintainer http://www.ffmpeg.org
- Previous message: [FFmpeg-devel] [RCF] lavfi aspect ratio setting path
- Next message: [FFmpeg-devel] [RCF] lavfi aspect ratio setting path
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]