Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :

> We read a display aspect ratio from ARES atom because of several
> user-provided samples, I believe it makes sense to also write the
> width value into the ares atom depending on the aspect ratio.

Patch applied, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-23 22:38 GMT+02:00 Paul B Mahol :
> On 9/23/16, Paul B Mahol  wrote:
>> On 9/23/16, Carl Eugen Hoyos  wrote:
>>> 2016-09-20 19:02 GMT+02:00 Paul B Mahol :
>>>
 How do you have tested that this patch is correct?
>>>
>>> Remuxing the sample from ticket #2125 is broken without this patch.
>>
>> Have you tested with QuickTime player?

Yes, how did you test?

> Also doesnt this allows division by zero?

Locally changed to ">0".

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Paul B Mahol]

On 9/23/16, Paul B Mahol  wrote:
> On 9/23/16, Carl Eugen Hoyos  wrote:
>> 2016-09-20 19:02 GMT+02:00 Paul B Mahol :
>>
>>> How do you have tested that this patch is correct?
>>
>> Remuxing the sample from ticket #2125 is broken without this patch.
>
> Have you tested with QuickTime player?
>

Also doesnt this allows division by zero?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Paul B Mahol]

On 9/23/16, Carl Eugen Hoyos  wrote:
> 2016-09-20 19:02 GMT+02:00 Paul B Mahol :
>
>> How do you have tested that this patch is correct?
>
> Remuxing the sample from ticket #2125 is broken without this patch.

Have you tested with QuickTime player?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-20 19:02 GMT+02:00 Paul B Mahol :

> How do you have tested that this patch is correct?

Remuxing the sample from ticket #2125 is broken without this patch.

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

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Paul B Mahol]

On 9/20/16, Carl Eugen Hoyos  wrote:
> 2016-09-20 18:47 GMT+02:00 Paul B Mahol :
>> On 9/20/16, Carl Eugen Hoyos  wrote:
>>> 2016-09-20 17:42 GMT+02:00 Paul B Mahol :
 On 9/20/16, Carl Eugen Hoyos  wrote:
> 2016-09-20 17:26 GMT+02:00 Paul B Mahol :
>> On 9/20/16, Carl Eugen Hoyos  wrote:
>>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
 Hi!

 We read a display aspect ratio from ARES atom because of several
 user-provided samples, I believe it makes sense to also write the
 width value into the ares atom depending on the aspect ratio.
>>>
>>> Ping.
>>
>> What about height?
>
> I thought height is already set correctly, is that wrong?

 So, height is not rescaled, but width is?
>>>
>>> It is rescaled for interlaced video iirc.
>>>
 This does not make sense.
>>>
>>> What do you suggest?
>>
>> Please stop sending incorrect patches.
>
> Please be slightly more constructive:
> How can I reproduce the issue you see with the patch I sent?

How do you have tested that this patch is correct?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-20 18:47 GMT+02:00 Paul B Mahol :
> On 9/20/16, Carl Eugen Hoyos  wrote:
>> 2016-09-20 17:42 GMT+02:00 Paul B Mahol :
>>> On 9/20/16, Carl Eugen Hoyos  wrote:
 2016-09-20 17:26 GMT+02:00 Paul B Mahol :
> On 9/20/16, Carl Eugen Hoyos  wrote:
>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
>>> Hi!
>>>
>>> We read a display aspect ratio from ARES atom because of several
>>> user-provided samples, I believe it makes sense to also write the
>>> width value into the ares atom depending on the aspect ratio.
>>
>> Ping.
>
> What about height?

 I thought height is already set correctly, is that wrong?
>>>
>>> So, height is not rescaled, but width is?
>>
>> It is rescaled for interlaced video iirc.
>>
>>> This does not make sense.
>>
>> What do you suggest?
>
> Please stop sending incorrect patches.

Please be slightly more constructive:
How can I reproduce the issue you see with the patch I sent?

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

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Paul B Mahol]

On 9/20/16, Carl Eugen Hoyos  wrote:
> 2016-09-20 17:42 GMT+02:00 Paul B Mahol :
>> On 9/20/16, Carl Eugen Hoyos  wrote:
>>> 2016-09-20 17:26 GMT+02:00 Paul B Mahol :
 On 9/20/16, Carl Eugen Hoyos  wrote:
> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
>> Hi!
>>
>> We read a display aspect ratio from ARES atom because of several
>> user-provided samples, I believe it makes sense to also write the
>> width value into the ares atom depending on the aspect ratio.
>
> Ping.

 What about height?
>>>
>>> I thought height is already set correctly, is that wrong?
>>
>> So, height is not rescaled, but width is?
>
> It is rescaled for interlaced video iirc.
>
>> This does not make sense.
>
> What do you suggest?

Please stop sending incorrect patches.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-20 17:42 GMT+02:00 Paul B Mahol :
> On 9/20/16, Carl Eugen Hoyos  wrote:
>> 2016-09-20 17:26 GMT+02:00 Paul B Mahol :
>>> On 9/20/16, Carl Eugen Hoyos  wrote:
 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
> Hi!
>
> We read a display aspect ratio from ARES atom because of several
> user-provided samples, I believe it makes sense to also write the
> width value into the ares atom depending on the aspect ratio.

 Ping.
>>>
>>> What about height?
>>
>> I thought height is already set correctly, is that wrong?
>
> So, height is not rescaled, but width is?

It is rescaled for interlaced video iirc.

> This does not make sense.

What do you suggest?

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

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Paul B Mahol]

On 9/20/16, Carl Eugen Hoyos  wrote:
> 2016-09-20 17:26 GMT+02:00 Paul B Mahol :
>> On 9/20/16, Carl Eugen Hoyos  wrote:
>>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
 Hi!

 We read a display aspect ratio from ARES atom because of several
 user-provided samples, I believe it makes sense to also write the
 width value into the ares atom depending on the aspect ratio.
>>>
>>> Ping.
>>
>> What about height?
>
> I thought height is already set correctly, is that wrong?

So, height is not rescaled, but width is?

This does not make sense.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-20 17:26 GMT+02:00 Paul B Mahol :
> On 9/20/16, Carl Eugen Hoyos  wrote:
>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
>>> Hi!
>>>
>>> We read a display aspect ratio from ARES atom because of several
>>> user-provided samples, I believe it makes sense to also write the
>>> width value into the ares atom depending on the aspect ratio.
>>
>> Ping.
>
> What about height?

I thought height is already set correctly, is that wrong?

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

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Paul B Mahol]

On 9/20/16, Carl Eugen Hoyos  wrote:
> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
>> Hi!
>>
>> We read a display aspect ratio from ARES atom because of several
>> user-provided samples, I believe it makes sense to also write the
>> width value into the ares atom depending on the aspect ratio.
>
> Ping.

What about height?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos :
> Hi!
>
> We read a display aspect ratio from ARES atom because of several
> user-provided samples, I believe it makes sense to also write the
> width value into the ares atom depending on the aspect ratio.

Ping.

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

[FFmpeg-devel] [PATCH]lavf/movenc: Put correct display aspect ratio in ARES atom

[Carl Eugen Hoyos]

Hi!

We read a display aspect ratio from ARES atom because of several 
user-provided samples, I believe it makes sense to also write the 
width value into the ares atom depending on the aspect ratio.

Please review, Carl Eugen
From ba2a97d8ff012895e39389dee65c075fe7a20f5c Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Mon, 19 Sep 2016 13:24:44 +0200
Subject: [PATCH] lavf/movenc: Put correct display aspect ratio in ARES atom.

---
 libavformat/movenc.c   |6 +-
 tests/ref/vsynth/vsynth1-dnxhd-1080i   |2 +-
 tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit |2 +-
 tests/ref/vsynth/vsynth1-dnxhd-1080i-colr  |2 +-
 tests/ref/vsynth/vsynth2-dnxhd-1080i   |2 +-
 tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit |2 +-
 tests/ref/vsynth/vsynth2-dnxhd-1080i-colr  |2 +-
 tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit |2 +-
 tests/ref/vsynth/vsynth3-dnxhd-1080i-colr  |2 +-
 tests/ref/vsynth/vsynth_lena-dnxhd-1080i   |2 +-
 tests/ref/vsynth/vsynth_lena-dnxhd-1080i-10bit |2 +-
 tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr  |2 +-
 12 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index aa4a076..fef884d 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1070,6 +1070,7 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack 
*track)
 int i;
 int interlaced;
 int cid;
+int display_width = track->par->width;
 
 if (track->vos_data && track->vos_len > 0x29) {
 if (ff_dnxhd_parse_header_prefix(track->vos_data) != 0) {
@@ -1121,7 +1122,10 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack 
*track)
 ffio_wfourcc(pb, "ARES");
 ffio_wfourcc(pb, "0001");
 avio_wb32(pb, cid); /* dnxhd cid, some id ? */
-avio_wb32(pb, track->par->width);
+if (   track->par->sample_aspect_ratio.num >= 0
+&& track->par->sample_aspect_ratio.den >= 0)
+display_width = display_width * track->par->sample_aspect_ratio.num / 
track->par->sample_aspect_ratio.den;
+avio_wb32(pb, display_width);
 /* values below are based on samples created with quicktime and avid 
codecs */
 if (interlaced) {
 avio_wb32(pb, track->par->height / 2);
diff --git a/tests/ref/vsynth/vsynth1-dnxhd-1080i 
b/tests/ref/vsynth/vsynth1-dnxhd-1080i
index 02f989f..8db0548 100644
--- a/tests/ref/vsynth/vsynth1-dnxhd-1080i
+++ b/tests/ref/vsynth/vsynth1-dnxhd-1080i
@@ -1,4 +1,4 @@
-a0234e0a8516d958f423b119aa9e35c4 *tests/data/fate/vsynth1-dnxhd-1080i.mov
+a6ffa1127be4d24536d5a131e0bd9f9b *tests/data/fate/vsynth1-dnxhd-1080i.mov
 3031911 tests/data/fate/vsynth1-dnxhd-1080i.mov
 fed9ed2a5179c9df0ef58772b025e303 
*tests/data/fate/vsynth1-dnxhd-1080i.out.rawvideo
 stddev:6.18 PSNR: 32.31 MAXDIFF:   64 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit 
b/tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit
index dd96e14..ece6e9e 100644
--- a/tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit
+++ b/tests/ref/vsynth/vsynth1-dnxhd-1080i-10bit
@@ -1,4 +1,4 @@
-f562845d1848bf5d3e524b418b742e01 *tests/data/fate/vsynth1-dnxhd-1080i-10bit.mov
+0b813077a8e61e2b776c25e028a25646 *tests/data/fate/vsynth1-dnxhd-1080i-10bit.mov
 4588391 tests/data/fate/vsynth1-dnxhd-1080i-10bit.mov
 31032fcb7e6af79daaac02288254c6d6 
*tests/data/fate/vsynth1-dnxhd-1080i-10bit.out.rawvideo
 stddev:5.69 PSNR: 33.02 MAXDIFF:   55 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr 
b/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr
index ac42966..8ca97ae 100644
--- a/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr
+++ b/tests/ref/vsynth/vsynth1-dnxhd-1080i-colr
@@ -1,4 +1,4 @@
-5fccdb16c0f14dea1b6b603bac90b97e *tests/data/fate/vsynth1-dnxhd-1080i-colr.mov
+a83febe7beb965a2d7df1b277ed55f33 *tests/data/fate/vsynth1-dnxhd-1080i-colr.mov
 3031929 tests/data/fate/vsynth1-dnxhd-1080i-colr.mov
 6f2d5429ffc4529a76acfeb28b560542 
*tests/data/fate/vsynth1-dnxhd-1080i-colr.out.rawvideo
 stddev:5.65 PSNR: 33.09 MAXDIFF:   55 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth2-dnxhd-1080i 
b/tests/ref/vsynth/vsynth2-dnxhd-1080i
index eabb6a2..e446d65 100644
--- a/tests/ref/vsynth/vsynth2-dnxhd-1080i
+++ b/tests/ref/vsynth/vsynth2-dnxhd-1080i
@@ -1,4 +1,4 @@
-2b75889122f8d918e1b068d128b618ca *tests/data/fate/vsynth2-dnxhd-1080i.mov
+3765c7d562250a83a26f4cafaad49325 *tests/data/fate/vsynth2-dnxhd-1080i.mov
 3031911 tests/data/fate/vsynth2-dnxhd-1080i.mov
 e941d2587cfeccddc450da7f41f7f911 
*tests/data/fate/vsynth2-dnxhd-1080i.out.rawvideo
 stddev:1.50 PSNR: 44.56 MAXDIFF:   31 bytes:  7603200/   760320
diff --git a/tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit 
b/tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit
index 3361c93..e811e38 100644
--- a/tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit
+++ b/tests/ref/vsynth/vsynth2-dnxhd-1080i-10bit
@@ -1,4 +1,4 @@
-514607eecfd9004aa4da1d216f7620ce