| Message ID | 201609191332.54592.cehoyos@ag.or.at |
|---|---|
| State | Accepted |
| Headers | show |
On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > Hi! > > Attached patch fixes the aspect ratio for the sample from > ticket #5325 / #2125. > > The fate test changes because the fate sample was written > by FFmpeg with an incorrect aspect ratio. > > Please comment, Carl Eugen > > From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <cehoyos@ag.or.at> > Date: Mon, 19 Sep 2016 13:30:06 +0200 > Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. > > Fixes ticket #2125. > Fixes ticket #5325. Feel free to spread lies to logs, it that helps your ego.
2016-09-19 13:37 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >> Hi! >> >> Attached patch fixes the aspect ratio for the sample from >> ticket #5325 / #2125. >> >> The fate test changes because the fate sample was written >> by FFmpeg with an incorrect aspect ratio. >> >> Please comment, Carl Eugen >> > >> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >> Date: Mon, 19 Sep 2016 13:30:06 +0200 >> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >> >> Fixes ticket #2125. >> Fixes ticket #5325. > > Feel free to spread lies to logs, it that helps your ego. I don't think I can follow you, sorry. Carl Eugen
On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-19 13:37 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >>> Hi! >>> >>> Attached patch fixes the aspect ratio for the sample from >>> ticket #5325 / #2125. >>> >>> The fate test changes because the fate sample was written >>> by FFmpeg with an incorrect aspect ratio. >>> >>> Please comment, Carl Eugen >>> >> >>> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>> Date: Mon, 19 Sep 2016 13:30:06 +0200 >>> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >>> >>> Fixes ticket #2125. >>> Fixes ticket #5325. >> >> Feel free to spread lies to logs, it that helps your ego. > > I don't think I can follow you, sorry. How you can claim that you fixed those two tickets?
2016-09-19 13:57 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-19 13:37 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >>> On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >>>> Hi! >>>> >>>> Attached patch fixes the aspect ratio for the sample from >>>> ticket #5325 / #2125. >>>> >>>> The fate test changes because the fate sample was written >>>> by FFmpeg with an incorrect aspect ratio. >>>> >>>> Please comment, Carl Eugen >>>> >>> >>>> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>>> Date: Mon, 19 Sep 2016 13:30:06 +0200 >>>> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >>>> >>>> Fixes ticket #2125. >>>> Fixes ticket #5325. >>> >>> Feel free to spread lies to logs, it that helps your ego. >> >> I don't think I can follow you, sorry. > > How you can claim that you fixed those two tickets? Afaict, the file is not decoded correctly by current FFmpeg. It looks better with the patch in this thread. Do you disagree? Carl Eugen
On Mon, Sep 19, 2016 at 01:37:10PM +0200, Paul B Mahol wrote: > On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > > Hi! > > > > Attached patch fixes the aspect ratio for the sample from > > ticket #5325 / #2125. > > > > The fate test changes because the fate sample was written > > by FFmpeg with an incorrect aspect ratio. > > > > Please comment, Carl Eugen > > > > > From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 > > From: Carl Eugen Hoyos <cehoyos@ag.or.at> > > Date: Mon, 19 Sep 2016 13:30:06 +0200 > > Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. > > > > Fixes ticket #2125. > > Fixes ticket #5325. > > Feel free to spread lies to logs, it that helps your ego. Please don't use such aggressive statements. It might just be a ticket number error here. From the code of conduct: "Do not assume malice for things that can be attributed to incompetence. Even if it is malice, it’s rarely good to start with that as initial assumption." Regards,
2016-09-19 14:00 GMT+02:00 Clément Bœsch <u@pkh.me>: > On Mon, Sep 19, 2016 at 01:37:10PM +0200, Paul B Mahol wrote: >> On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: >> > Hi! >> > >> > Attached patch fixes the aspect ratio for the sample from >> > ticket #5325 / #2125. >> > >> > The fate test changes because the fate sample was written >> > by FFmpeg with an incorrect aspect ratio. >> > >> > Please comment, Carl Eugen >> > >> >> > From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >> > From: Carl Eugen Hoyos <cehoyos@ag.or.at> >> > Date: Mon, 19 Sep 2016 13:30:06 +0200 >> > Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >> > >> > Fixes ticket #2125. >> > Fixes ticket #5325. >> >> Feel free to spread lies to logs, it that helps your ego. > > Please don't use such aggressive statements. It might just be a ticket > number error here. Why do you believe that there is an error? > From the code of conduct: "Do not assume malice for things that can be > attributed to incompetence. Even if it is malice, it’s rarely good to > start with that as initial assumption." I thought this is not relevant anymore? Carl Eugen
On Mon, Sep 19, 2016 at 01:32:54PM +0200, Carl Eugen Hoyos wrote: > Hi! > > Attached patch fixes the aspect ratio for the sample from > ticket #5325 / #2125. > > The fate test changes because the fate sample was written > by FFmpeg with an incorrect aspect ratio. > > Please comment, Carl Eugen > From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos <cehoyos@ag.or.at> > Date: Mon, 19 Sep 2016 13:30:06 +0200 > Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. > > Fixes ticket #2125. > Fixes ticket #5325. It fixes aspect ratio from samples from these tickets, it looks unrelated to the original issue. Regards,
2016-09-19 14:02 GMT+02:00 Clément Bœsch <u@pkh.me>: > On Mon, Sep 19, 2016 at 01:32:54PM +0200, Carl Eugen Hoyos wrote: >> Hi! >> >> Attached patch fixes the aspect ratio for the sample from >> ticket #5325 / #2125. >> >> The fate test changes because the fate sample was written >> by FFmpeg with an incorrect aspect ratio. >> >> Please comment, Carl Eugen > >> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >> Date: Mon, 19 Sep 2016 13:30:06 +0200 >> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >> > >> Fixes ticket #2125. >> Fixes ticket #5325. > > It fixes aspect ratio from samples from these tickets, it looks unrelated > to the original issue. We seem to have very different interpretations of "unrelated" but changed locally to "Fixes aspect ratio of sample in ticket ...." Carl Eugen
On Mon, Sep 19, 2016 at 02:01:17PM +0200, Carl Eugen Hoyos wrote: > 2016-09-19 14:00 GMT+02:00 Clément Bœsch <u@pkh.me>: > > On Mon, Sep 19, 2016 at 01:37:10PM +0200, Paul B Mahol wrote: > >> On 9/19/16, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > >> > Hi! > >> > > >> > Attached patch fixes the aspect ratio for the sample from > >> > ticket #5325 / #2125. > >> > > >> > The fate test changes because the fate sample was written > >> > by FFmpeg with an incorrect aspect ratio. > >> > > >> > Please comment, Carl Eugen > >> > > >> > >> > From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 > >> > From: Carl Eugen Hoyos <cehoyos@ag.or.at> > >> > Date: Mon, 19 Sep 2016 13:30:06 +0200 > >> > Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. > >> > > >> > Fixes ticket #2125. > >> > Fixes ticket #5325. > >> > >> Feel free to spread lies to logs, it that helps your ego. > > > > Please don't use such aggressive statements. It might just be a ticket > > number error here. > > Why do you believe that there is an error? > Because I'm assuming you made a mistake some way or another. It does look like wording instead of ticket number. > > From the code of conduct: "Do not assume malice for things that can be > > attributed to incompetence. Even if it is malice, it’s rarely good to > > start with that as initial assumption." > > I thought this is not relevant anymore? > Why do you believe that?
2016-09-19 14:03 GMT+02:00 Clément Bœsch <u@pkh.me>: >> > From the code of conduct: "Do not assume malice for things that can be >> > attributed to incompetence. Even if it is malice, it’s rarely good to >> > start with that as initial assumption." >> >> I thought this is not relevant anymore? > > Why do you believe that? Last week a long-time contributor was attacked repeatedly and no steps were taken so I was hoping that these days of political correctness are over. Was I wrong? Carl Eugen
On Mon, Sep 19, 2016 at 02:03:22PM +0200, Carl Eugen Hoyos wrote: > 2016-09-19 14:02 GMT+02:00 Clément Bœsch <u@pkh.me>: > > On Mon, Sep 19, 2016 at 01:32:54PM +0200, Carl Eugen Hoyos wrote: > >> Hi! > >> > >> Attached patch fixes the aspect ratio for the sample from > >> ticket #5325 / #2125. > >> > >> The fate test changes because the fate sample was written > >> by FFmpeg with an incorrect aspect ratio. > >> > >> Please comment, Carl Eugen > > > >> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 > >> From: Carl Eugen Hoyos <cehoyos@ag.or.at> > >> Date: Mon, 19 Sep 2016 13:30:06 +0200 > >> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. > >> > > > >> Fixes ticket #2125. > >> Fixes ticket #5325. > > > > It fixes aspect ratio from samples from these tickets, it looks unrelated > > to the original issue. > > We seem to have very different interpretations of "unrelated" but changed > locally to "Fixes aspect ratio of sample in ticket ...." > Thanks
On Mon, Sep 19, 2016 at 02:04:43PM +0200, Carl Eugen Hoyos wrote: > 2016-09-19 14:03 GMT+02:00 Clément Bœsch <u@pkh.me>: > > >> > From the code of conduct: "Do not assume malice for things that can be > >> > attributed to incompetence. Even if it is malice, it’s rarely good to > >> > start with that as initial assumption." > >> > >> I thought this is not relevant anymore? > > > > Why do you believe that? > > Last week a long-time contributor was attacked repeatedly > and no steps were taken so I was hoping that these days > of political correctness are over. > > Was I wrong? > I missed that so I have no idea what you are referring to, I'm sorry. Also, I'm not a police officer, I just happened to read that thread that just popped in my mailbox and wanted to avoid any escalation. Regards,
On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-19 14:02 GMT+02:00 Clement Boesch <u@pkh.me>: >> On Mon, Sep 19, 2016 at 01:32:54PM +0200, Carl Eugen Hoyos wrote: >>> Hi! >>> >>> Attached patch fixes the aspect ratio for the sample from >>> ticket #5325 / #2125. >>> >>> The fate test changes because the fate sample was written >>> by FFmpeg with an incorrect aspect ratio. >>> >>> Please comment, Carl Eugen >> >>> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>> Date: Mon, 19 Sep 2016 13:30:06 +0200 >>> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >>> >> >>> Fixes ticket #2125. >>> Fixes ticket #5325. >> >> It fixes aspect ratio from samples from these tickets, it looks unrelated >> to the original issue. > > We seem to have very different interpretations of "unrelated" but changed > locally to "Fixes aspect ratio of sample in ticket ...." Maybe this patch with above locally changed text is correct. But it does change nothing with sample from ticket #5325. As that sample have same sar as before this patch. So it is another lie.
On 9/19/16, Paul B Mahol <onemda@gmail.com> wrote: > On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-19 14:02 GMT+02:00 Clement Boesch <u@pkh.me>: >>> On Mon, Sep 19, 2016 at 01:32:54PM +0200, Carl Eugen Hoyos wrote: >>>> Hi! >>>> >>>> Attached patch fixes the aspect ratio for the sample from >>>> ticket #5325 / #2125. >>>> >>>> The fate test changes because the fate sample was written >>>> by FFmpeg with an incorrect aspect ratio. >>>> >>>> Please comment, Carl Eugen >>> >>>> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>>> Date: Mon, 19 Sep 2016 13:30:06 +0200 >>>> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >>>> >>> >>>> Fixes ticket #2125. >>>> Fixes ticket #5325. >>> >>> It fixes aspect ratio from samples from these tickets, it looks >>> unrelated >>> to the original issue. >> >> We seem to have very different interpretations of "unrelated" but changed >> locally to "Fixes aspect ratio of sample in ticket ...." > > Maybe this patch with above locally changed text is correct. > > But it does change nothing with sample from ticket #5325. As that sample > have same sar as before this patch. > > So it is another lie. > Ignore that one above, patck with corrected text is ok.
On 9/19/16, Paul B Mahol <onemda@gmail.com> wrote: > On 9/19/16, Paul B Mahol <onemda@gmail.com> wrote: >> On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> 2016-09-19 14:02 GMT+02:00 Clement Boesch <u@pkh.me>: >>>> On Mon, Sep 19, 2016 at 01:32:54PM +0200, Carl Eugen Hoyos wrote: >>>>> Hi! >>>>> >>>>> Attached patch fixes the aspect ratio for the sample from >>>>> ticket #5325 / #2125. >>>>> >>>>> The fate test changes because the fate sample was written >>>>> by FFmpeg with an incorrect aspect ratio. >>>>> >>>>> Please comment, Carl Eugen >>>> >>>>> From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 >>>>> From: Carl Eugen Hoyos <cehoyos@ag.or.at> >>>>> Date: Mon, 19 Sep 2016 13:30:06 +0200 >>>>> Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. >>>>> >>>> >>>>> Fixes ticket #2125. >>>>> Fixes ticket #5325. >>>> >>>> It fixes aspect ratio from samples from these tickets, it looks >>>> unrelated >>>> to the original issue. >>> >>> We seem to have very different interpretations of "unrelated" but >>> changed >>> locally to "Fixes aspect ratio of sample in ticket ...." >> >> Maybe this patch with above locally changed text is correct. >> >> But it does change nothing with sample from ticket #5325. As that sample >> have same sar as before this patch. >> >> So it is another lie. >> > > Ignore that one above, patck with corrected text is ok. > title should mentioned what kind of aspect ratio this is about. As I thought it is about sar.
2016-09-19 14:57 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > title should mentioned what kind of aspect ratio this is about. > As I thought it is about sar. I don't understand: FFmpeg only knows about sample aspect ratio (also known as pixel aspect ratio), display aspect ratio is only shown on the console for convenience. Without this patch, sar is 1:1 and (I suspect) the sample is not decoded correctly, with my patch sar is 4:3 and the sample looks better. What do I miss? Carl Eugen
On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-19 14:57 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > >> title should mentioned what kind of aspect ratio this is about. >> As I thought it is about sar. > > I don't understand: FFmpeg only knows about sample aspect > ratio (also known as pixel aspect ratio), display aspect ratio is > only shown on the console for convenience. > Without this patch, sar is 1:1 and (I suspect) the sample is not > decoded correctly, with my patch sar is 4:3 and the sample > looks better. > > What do I miss? ARES atom does not store sar. And this patch sets dar for AVdn.
2016-09-19 15:27 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/19/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-09-19 14:57 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> >>> title should mentioned what kind of aspect ratio this is about. >>> As I thought it is about sar. >> >> I don't understand: FFmpeg only knows about sample aspect >> ratio (also known as pixel aspect ratio), display aspect ratio is >> only shown on the console for convenience. >> Without this patch, sar is 1:1 and (I suspect) the sample is not >> decoded correctly, with my patch sar is 4:3 and the sample >> looks better. >> >> What do I miss? > > ARES atom does not store sar. And this patch sets dar for AVdn. Which is converted to sar (the only thing libavcodec cares about and the only thing FFmpeg exports) in utils.c. Carl Eugen
2016-09-19 14:57 GMT+02:00 Paul B Mahol <onemda@gmail.com>: > On 9/19/16, Paul B Mahol <onemda@gmail.com> wrote: >> Ignore that one above, patck with corrected text is ok. > > title should mentioned what kind of aspect ratio this is about. As I > thought it is about sar. Log message changed locally to "lavf/mov: Read display aspect ratio from ares atom also for dnxhd." Carl Eugen
On 9/20/16, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-09-19 14:57 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> On 9/19/16, Paul B Mahol <onemda@gmail.com> wrote: > >>> Ignore that one above, patck with corrected text is ok. >> >> title should mentioned what kind of aspect ratio this is about. As I >> thought it is about sar. > > Log message changed locally to "lavf/mov: Read display > aspect ratio from ares atom also for dnxhd." LGTM
2016-09-23 1:08 GMT+02:00 Paul B Mahol <onemda@gmail.com>: >> Log message changed locally to "lavf/mov: Read display >> aspect ratio from ares atom also for dnxhd." > > LGTM Patch applied. Carl Eugen
diff --git a/libavformat/mov.c b/libavformat/mov.c index 6e80b93..ce24e2e 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1486,7 +1486,8 @@ static int mov_read_ares(MOVContext *c, AVIOContext *pb, MOVAtom atom) if (cid == 0xd4d || cid == 0xd4e) par->width = 1440; return 0; - } else if (par->codec_tag == MKTAG('A', 'V', 'd', '1') && + } else if ((par->codec_tag == MKTAG('A', 'V', 'd', '1') || + par->codec_tag == MKTAG('A', 'V', 'd', 'n')) && atom.size >= 24) { int num, den; avio_skip(pb, 12); diff --git a/tests/ref/fate/dnxhd-mbaff b/tests/ref/fate/dnxhd-mbaff index d694cfe..8e95227 100644 --- a/tests/ref/fate/dnxhd-mbaff +++ b/tests/ref/fate/dnxhd-mbaff @@ -2,5 +2,5 @@ #media_type 0: video #codec_id 0: rawvideo #dimensions 0: 1440x1080 -#sar 0: 4/3 +#sar 0: 1/1 0, 0, 0, 1, 6220800, 0xe78198c0
Hi! Attached patch fixes the aspect ratio for the sample from ticket #5325 / #2125. The fate test changes because the fate sample was written by FFmpeg with an incorrect aspect ratio. Please comment, Carl Eugen From 0553b0adfee87401854f0313dbcf386f2fb7ae68 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <cehoyos@ag.or.at> Date: Mon, 19 Sep 2016 13:30:06 +0200 Subject: [PATCH] lavf/mov: Read aspect ratio from ares atom for dnxhd. Fixes ticket #2125. Fixes ticket #5325. --- libavformat/mov.c | 3 ++- tests/ref/fate/dnxhd-mbaff | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)