| Message ID | 201609191328.11052.cehoyos@ag.or.at |
|---|---|
| State | Accepted |
| Headers | show |
2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: > 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
On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >> 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?
2016-09-20 17:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >>> 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
On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-20 17:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >>>> 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.
2016-09-20 17:42 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-20 17:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >>>>> 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
On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-20 17:42 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> 2016-09-20 17:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>>> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >>>>>> 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.
2016-09-20 18:47 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-20 17:42 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>> 2016-09-20 17:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>>>> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>>>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >>>>>>> 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
On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-20 18:47 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> 2016-09-20 17:42 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>>> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>>> 2016-09-20 17:26 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>>>>> On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>>>>>> 2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: >>>>>>>> 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?
2016-09-20 19:02 GMT+02:00 Paul B Mahol <onemda@gmail.com>:
> How do you have tested that this patch is correct?
Remuxing the sample from ticket #2125 is broken without this patch.
Carl Eugen
On 9/23/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-20 19:02 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > >> 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?
On 9/23/16, Paul B Mahol <onemda@gmail.com> wrote: > On 9/23/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-20 19:02 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> >>> 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?
2016-09-23 22:38 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/23/16, Paul B Mahol <onemda@gmail.com> wrote: >> On 9/23/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> 2016-09-20 19:02 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>> >>>> 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
2016-09-19 13:28 GMT+02:00 Carl Eugen Hoyos <cehoyos@ag.or.at>: > 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
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 *tests/data/fate/vsynth2-dnxhd-1080i-10bit.mov +77ff954dcbfd2c8a03e9f75484aa90ca *tests/data/fate/vsynth2-dnxhd-1080i-10bit.mov 4588391 tests/data/fate/vsynth2-dnxhd-1080i-10bit.mov e4ca9be476869afb94962d945f90bdf6 *tests/data/fate/vsynth2-dnxhd-1080i-10bit.out.rawvideo stddev: 1.57 PSNR: 44.18 MAXDIFF: 33 bytes: 7603200/ 760320 diff --git a/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr b/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr index 06731a8..c908274 100644 --- a/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr +++ b/tests/ref/vsynth/vsynth2-dnxhd-1080i-colr @@ -1,4 +1,4 @@ -f9827e9867b0ea4f7585d8e362a58413 *tests/data/fate/vsynth2-dnxhd-1080i-colr.mov +ee5ebb531d6ce1f9e69f71ed99f06f9a *tests/data/fate/vsynth2-dnxhd-1080i-colr.mov 3031929 tests/data/fate/vsynth2-dnxhd-1080i-colr.mov ec40a8014b819d02951b2f06bee7b514 *tests/data/fate/vsynth2-dnxhd-1080i-colr.out.rawvideo stddev: 1.54 PSNR: 44.33 MAXDIFF: 33 bytes: 7603200/ 760320 diff --git a/tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit b/tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit index 006af6c..de5aef4 100644 --- a/tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit +++ b/tests/ref/vsynth/vsynth3-dnxhd-1080i-10bit @@ -1,4 +1,4 @@ -dea8862f8ae9fb03f665f358dde75962 *tests/data/fate/vsynth3-dnxhd-1080i-10bit.mov +d743bc08dea7d5f25e2a26c94273e656 *tests/data/fate/vsynth3-dnxhd-1080i-10bit.mov 4588391 tests/data/fate/vsynth3-dnxhd-1080i-10bit.mov c192f36ef8687e56c72a3dc416c7e191 *tests/data/fate/vsynth3-dnxhd-1080i-10bit.out.rawvideo stddev: 6.92 PSNR: 31.32 MAXDIFF: 50 bytes: 86700/ 8670 diff --git a/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr b/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr index 8d7d3b6..8c3152c 100644 --- a/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr +++ b/tests/ref/vsynth/vsynth3-dnxhd-1080i-colr @@ -1,4 +1,4 @@ -ee7a70832f37793b62642f770d988bdb *tests/data/fate/vsynth3-dnxhd-1080i-colr.mov +6ca6c17275ed8d80e7e9f09a9621cb76 *tests/data/fate/vsynth3-dnxhd-1080i-colr.mov 3031929 tests/data/fate/vsynth3-dnxhd-1080i-colr.mov f907fd2d48bedbc5283fbfc3fb9f61a0 *tests/data/fate/vsynth3-dnxhd-1080i-colr.out.rawvideo stddev: 6.92 PSNR: 31.32 MAXDIFF: 50 bytes: 86700/ 8670 diff --git a/tests/ref/vsynth/vsynth_lena-dnxhd-1080i b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i index 16702db..ac384f5 100644 --- a/tests/ref/vsynth/vsynth_lena-dnxhd-1080i +++ b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i @@ -1,4 +1,4 @@ -f7412afbcb4454692f7492f6710189e3 *tests/data/fate/vsynth_lena-dnxhd-1080i.mov +3818abedd4160b26a8919d24edd37733 *tests/data/fate/vsynth_lena-dnxhd-1080i.mov 3031911 tests/data/fate/vsynth_lena-dnxhd-1080i.mov 7d0ca92f12711535d57eff3609462b31 *tests/data/fate/vsynth_lena-dnxhd-1080i.out.rawvideo stddev: 1.29 PSNR: 45.87 MAXDIFF: 22 bytes: 7603200/ 760320 diff --git a/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-10bit b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-10bit index 109e3d5..cd999e2 100644 --- a/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-10bit +++ b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-10bit @@ -1,4 +1,4 @@ -72144676d0c6e320ff2c9b28bc3e4fa2 *tests/data/fate/vsynth_lena-dnxhd-1080i-10bit.mov +f6dc6ae9ce4c6f78bc99cf4eb2356098 *tests/data/fate/vsynth_lena-dnxhd-1080i-10bit.mov 4588391 tests/data/fate/vsynth_lena-dnxhd-1080i-10bit.mov f2dc4375c58e0406d442e0cb28573e91 *tests/data/fate/vsynth_lena-dnxhd-1080i-10bit.out.rawvideo stddev: 1.36 PSNR: 45.40 MAXDIFF: 22 bytes: 7603200/ 760320 diff --git a/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr index 8e43a3f..5d7e6b1 100644 --- a/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr +++ b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr @@ -1,4 +1,4 @@ -5ba3ddb58b10e5f0069cb4f82d594695 *tests/data/fate/vsynth_lena-dnxhd-1080i-colr.mov +360fe645539c74d1e3c94216e38829db *tests/data/fate/vsynth_lena-dnxhd-1080i-colr.mov 3031929 tests/data/fate/vsynth_lena-dnxhd-1080i-colr.mov ce4993a69ef55c8c4b18138716f17b6f *tests/data/fate/vsynth_lena-dnxhd-1080i-colr.out.rawvideo stddev: 1.33 PSNR: 45.59 MAXDIFF: 22 bytes: 7603200/ 760320
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 <cehoyos@ag.or.at> 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(-)