Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-17 Thread Michael Niedermayer
On Fri, Jan 16, 2015 at 08:48:59PM -0800, Philip Langdale wrote:
 On Fri, 16 Jan 2015 22:17:56 +0100
 Nicolas George geo...@nsup.org wrote:
 
  Le septidi 27 nivôse, an CCXXIII, Philip Langdale a écrit :
   Right. It is display aspect ratio, not sample aspect ratio. And then
   you have the 45/44 problem, unless that's somehow just affecting me.
  
  IIRC, the tests you ran were not accurate enough to know for sure. I
  suggest the following test case:
  
  - start with a 832 × 448 input (testsrc=s=832x448 should do the
  trick);
  
  - force darWidth / darHeight = 19 / 17 (patching the source directly
  seems like the simplest way of doing it for this kind of tests);
  
  - encode to H.265 elementary stream;
  
  - check the output file with the ffmpeg summary AND ffprobe
  -show_stream AND ffmpeg -vf showinfo -f null -.
  
  The numbers are selected with unambiguous prime factors.
  
  I do not have access to nvidia hardware, so I can not run the tests.
 
 Ok. I did this test and it produces correct results - SAR 133:221 which
 yields the correct final aspect ratio, but if I try and do a real world
 case, like my PAL DVD, it goes wrong - and Timo's patch goes wrong too,
 with this same weird 1.02 (45/44) scale factor.
 
 It's making my head hurt.
 
 So I feed in this input:
 
 Stream #0:0: Video: rawvideo (RGB[24] / 0x18424752), rgb24, 720x576
 [SAR 64:45 DAR 16:9], 25 tbr, 25 tbn, 25 tbc
 
 With libx264, I get the same SAR and DAR out.
 
 With nvenc, I get:
 
 sample_aspect_ratio=16:11
 display_aspect_ratio=20:11
 
 which is everything off by ~1.02.

this leaves the explanation that nvidias stuff is buggy as the
obvious explanation

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

Democracy is the form of government in which you can choose your dictator


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-17 Thread Nicolas George
L'octidi 28 nivôse, an CCXXIII, Kieran Kunhya a écrit :
 I haven't quite followed what's going on to decide whether Nvidia or
 FFmpeg is correct.

FFmpeg is correct, there is absolutely no doubt about it: This active
pixels nonsense is only relevant for certain very specific media, definitely
not when encoding testsrc and decoding the result to showinfo. Demuxers or
high-level tools may know when they are dealing with that kind of content,
but encoders do not.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-17 Thread Philip Langdale
On Sat, 17 Jan 2015 19:27:17 +
Kieran Kunhya kier...@obe.tv wrote:

 On 17 January 2015 at 18:14, Nicolas George geo...@nsup.org wrote:
  Le septidi 27 nivôse, an CCXXIII, Philip Langdale a écrit :
  On Fri, 16 Jan 2015 22:17:56 +0100
  Nicolas George geo...@nsup.org wrote:
  Ok. I did this test and it produces correct results - SAR 133:221
  which yields the correct final aspect ratio,
 
  This is a good start.
 
 but if I try and do a real
  world case, like my PAL DVD, it goes wrong - and Timo's patch goes
  wrong too, with this same weird 1.02 (45/44) scale factor.
 
  The 45/44 value looks like 704/720: the sign that some people long
  time ago in committees in the broadcast industry could not agree
  whether the aspect ratio applies to the whole image or only the
  part that is displayed on screen.
 
 The active picture width is 702 pixels. I would read the following
 thread and ETSI TS 101 154 for more information about SAR/DAR etc
 https://lists.libav.org/pipermail/ffmpeg-user/2010-March/024551.html
 
 I haven't quite followed what's going on to decide whether Nvidia or
 FFmpeg is correct.

For me, the problem is that the encoder is silently doing this aspect
ratio fiddling, whether I want it or not. If I want to transcode
material at the original size and aspect ratio, I should be able to. If
I, for whatever reason, want to adjust for active picture constraints
at encode time, I can set the SAR appropriately. So, ffmpeg applies
what you tell it to, the hardware adjusts what you tell it by 44/45 -
I'd call that wrong.

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-17 Thread Kieran Kunhya
On 17 January 2015 at 18:14, Nicolas George geo...@nsup.org wrote:
 Le septidi 27 nivôse, an CCXXIII, Philip Langdale a écrit :
 On Fri, 16 Jan 2015 22:17:56 +0100
 Nicolas George geo...@nsup.org wrote:
 Ok. I did this test and it produces correct results - SAR 133:221 which
 yields the correct final aspect ratio,

 This is a good start.

but if I try and do a real world
 case, like my PAL DVD, it goes wrong - and Timo's patch goes wrong too,
 with this same weird 1.02 (45/44) scale factor.

 The 45/44 value looks like 704/720: the sign that some people long time ago
 in committees in the broadcast industry could not agree whether the aspect
 ratio applies to the whole image or only the part that is displayed on
 screen.

The active picture width is 702 pixels. I would read the following
thread and ETSI TS 101 154 for more information about SAR/DAR etc
https://lists.libav.org/pipermail/ffmpeg-user/2010-March/024551.html

I haven't quite followed what's going on to decide whether Nvidia or
FFmpeg is correct.

Kieran
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-17 Thread Kieran Kunhya
 FFmpeg is correct, there is absolutely no doubt about it: This active
 pixels nonsense is only relevant for certain very specific media, definitely
 not when encoding testsrc and decoding the result to showinfo. Demuxers or
 high-level tools may know when they are dealing with that kind of content,
 but encoders do not.

This is ridiculous. It's an incredible double standard that FFmpeg
will fix severely broken files, yet the basics of digital video that
have been around for decades are just ignored because they are
inconvenient.

Testsrc is wrong then - note how downscaled HD test sequences like
parkjoy have 9 pixels of border when downscaled to SD - because it
follows the BT601 spec.

Where do you think 720x576 (or similar) content comes from? Magic
fairies that just happen to render at this resolution. No - it comes
from cameras that have an active picture of 702x576.

Kieran
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-16 Thread Philip Langdale

On 2015-01-15 23:48, Nicolas George wrote:

Le septidi 27 nivôse, an CCXXIII, Timo Rothenpieler a écrit :
+av_reduce(dw, dh, avctx-sample_aspect_ratio.num, 
avctx-sample_aspect_ratio.den, 4096);

+ctx-init_encode_params.darHeight = dw;
+ctx-init_encode_params.darWidth = dh;


Has this been actually tested? dar in the API does not seem to be the 
same

thing as sample_aspect_ratio in FFmpeg's API.


Right. It is display aspect ratio, not sample aspect ratio. And then you 
have the

45/44 problem, unless that's somehow just affecting me.

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-15 Thread Timo Rothenpieler
---
 libavcodec/nvenc.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index 90856be..57ca130 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -471,6 +471,7 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx)
 int i, num_mbs;
 int isLL = 0;
 int res = 0;
+int dw, dh;
 
 NvencContext *ctx = avctx-priv_data;
 NvencDynLoadFunctions *dl_fn = ctx-nvenc_dload_funcs;
@@ -564,8 +565,16 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx)
 ctx-init_encode_params.encodeGUID = NV_ENC_CODEC_H264_GUID;
 ctx-init_encode_params.encodeHeight = avctx-height;
 ctx-init_encode_params.encodeWidth = avctx-width;
-ctx-init_encode_params.darHeight = avctx-height;
-ctx-init_encode_params.darWidth = avctx-width;
+
+if (avctx-sample_aspect_ratio.num  avctx-sample_aspect_ratio.den) {
+av_reduce(dw, dh, avctx-sample_aspect_ratio.num, 
avctx-sample_aspect_ratio.den, 4096);
+ctx-init_encode_params.darHeight = dw;
+ctx-init_encode_params.darWidth = dh;
+} else {
+ctx-init_encode_params.darHeight = avctx-height;
+ctx-init_encode_params.darWidth = avctx-width;
+}
+
 ctx-init_encode_params.frameRateNum = avctx-time_base.den;
 ctx-init_encode_params.frameRateDen = avctx-time_base.num * 
avctx-ticks_per_frame;
 
-- 
2.2.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/nvenc: Handle non-square pixel aspect ratios

2015-01-15 Thread Nicolas George
Le septidi 27 nivôse, an CCXXIII, Timo Rothenpieler a écrit :
 +av_reduce(dw, dh, avctx-sample_aspect_ratio.num, 
 avctx-sample_aspect_ratio.den, 4096);
 +ctx-init_encode_params.darHeight = dw;
 +ctx-init_encode_params.darWidth = dh;

Has this been actually tested? dar in the API does not seem to be the same
thing as sample_aspect_ratio in FFmpeg's API.

Regards,

-- 
  Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel