diff mbox

[FFmpeg-devel] avformat/matroskaenc: use display aspect ratio for DisplayWidth and DisplayHeight when possible

Message ID 20161022203330.5416-1-jamrial@gmail.com
State Accepted
Commit 5cb57131d3bb2422858467828a5763646f004bbe
Headers show

Commit Message

James Almer Oct. 22, 2016, 8:33 p.m. UTC 
This avoids potential rounding errors and guarantees the source aspect
ratio is preserved.
Keep writing pixel values when Stereo 3D Mode is enabled and for WebM,
as the format doesn't support anything else.

This fixes ticket #5743, implementing the suggestion from ticket #5903.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/matroskaenc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

James Almer Oct. 28, 2016, 3:03 a.m. UTC | #1 
On 10/22/2016 5:33 PM, James Almer wrote:
> This avoids potential rounding errors and guarantees the source aspect
> ratio is preserved.
> Keep writing pixel values when Stereo 3D Mode is enabled and for WebM,
> as the format doesn't support anything else.
> 
> This fixes ticket #5743, implementing the suggestion from ticket #5903.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/matroskaenc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 03d5326..5790fe1 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1193,8 +1193,19 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>                  return AVERROR(EINVAL);
>              }
>              if (d_width != par->width || display_width_div != 1 || display_height_div != 1) {
> -                put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / display_width_div);
> -                put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / display_height_div);
> +                if (mkv->mode == MODE_WEBM || display_width_div != 1 || display_height_div != 1) {
> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / display_width_div);
> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / display_height_div);
> +                } else {
> +                    AVRational display_aspect_ratio;
> +                    av_reduce(&display_aspect_ratio.num, &display_aspect_ratio.den,
> +                              par->width  * (int64_t)st->sample_aspect_ratio.num,
> +                              par->height * (int64_t)st->sample_aspect_ratio.den,
> +                              1024 * 1024);
> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH,  display_aspect_ratio.num);
> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, display_aspect_ratio.den);
> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYUNIT, MATROSKA_VIDEO_DISPLAYUNIT_DAR);
> +                }
>              }
>          } else if (display_width_div != 1 || display_height_div != 1) {
>              put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , par->width / display_width_div);

Ping.
James Almer Nov. 2, 2016, 8:26 p.m. UTC | #2 
On 10/28/2016 12:03 AM, James Almer wrote:
> On 10/22/2016 5:33 PM, James Almer wrote:
>> This avoids potential rounding errors and guarantees the source aspect
>> ratio is preserved.
>> Keep writing pixel values when Stereo 3D Mode is enabled and for WebM,
>> as the format doesn't support anything else.
>>
>> This fixes ticket #5743, implementing the suggestion from ticket #5903.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavformat/matroskaenc.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 03d5326..5790fe1 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -1193,8 +1193,19 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>>                  return AVERROR(EINVAL);
>>              }
>>              if (d_width != par->width || display_width_div != 1 || display_height_div != 1) {
>> -                put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / display_width_div);
>> -                put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / display_height_div);
>> +                if (mkv->mode == MODE_WEBM || display_width_div != 1 || display_height_div != 1) {
>> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / display_width_div);
>> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / display_height_div);
>> +                } else {
>> +                    AVRational display_aspect_ratio;
>> +                    av_reduce(&display_aspect_ratio.num, &display_aspect_ratio.den,
>> +                              par->width  * (int64_t)st->sample_aspect_ratio.num,
>> +                              par->height * (int64_t)st->sample_aspect_ratio.den,
>> +                              1024 * 1024);
>> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH,  display_aspect_ratio.num);
>> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, display_aspect_ratio.den);
>> +                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYUNIT, MATROSKA_VIDEO_DISPLAYUNIT_DAR);
>> +                }
>>              }
>>          } else if (display_width_div != 1 || display_height_div != 1) {
>>              put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , par->width / display_width_div);
> 
> Ping.
> 

Applied
Carl Eugen Hoyos Nov. 2, 2016, 10:15 p.m. UTC | #3 
2016-10-22 22:33 GMT+02:00 James Almer <jamrial@gmail.com>:
> This avoids potential rounding errors and guarantees the source aspect
> ratio is preserved.
> Keep writing pixel values when Stereo 3D Mode is enabled and for WebM,
> as the format doesn't support anything else.
>
> This fixes ticket #5743, implementing the suggestion from ticket #5903.

Thank you for fixing this!

Carl Eugen
Kieran O Leary Nov. 2, 2016, 10:21 p.m. UTC | #4 
Hi

On 2 Nov 2016 10:15 p.m., "Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:
>

> >
> > This fixes ticket #5743, implementing the suggestion from ticket #5903.
>
> Thank you for fixing this!
>
> Carl Eugen
>

+1 Thanks James!
diff mbox

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 03d5326..5790fe1 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1193,8 +1193,19 @@  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
                 return AVERROR(EINVAL);
             }
             if (d_width != par->width || display_width_div != 1 || display_height_div != 1) {
-                put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / display_width_div);
-                put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / display_height_div);
+                if (mkv->mode == MODE_WEBM || display_width_div != 1 || display_height_div != 1) {
+                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / display_width_div);
+                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / display_height_div);
+                } else {
+                    AVRational display_aspect_ratio;
+                    av_reduce(&display_aspect_ratio.num, &display_aspect_ratio.den,
+                              par->width  * (int64_t)st->sample_aspect_ratio.num,
+                              par->height * (int64_t)st->sample_aspect_ratio.den,
+                              1024 * 1024);
+                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH,  display_aspect_ratio.num);
+                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, display_aspect_ratio.den);
+                    put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYUNIT, MATROSKA_VIDEO_DISPLAYUNIT_DAR);
+                }
             }
         } else if (display_width_div != 1 || display_height_div != 1) {
             put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , par->width / display_width_div);