diff mbox

[FFmpeg-devel] avformat/matroskaenc: don't rescale mastering metadata values

Message ID 20191003235220.1351-1-jamrial@gmail.com
State Accepted
Commit 3b4e9a31ea263830d6dfbc402c02e3c8d017094f
Headers show

Commit Message

James Almer Oct. 3, 2019, 11:52 p.m. UTC
The rescaling can be done at muxing/encoding time, for formats that require it.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/matroskadec.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

Comments

Carl Eugen Hoyos Oct. 4, 2019, 12:26 a.m. UTC | #1
Am Fr., 4. Okt. 2019 um 01:59 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> The rescaling can be done at muxing/encoding time, for formats that require it.

Doesn't this need a micro version bump?

Carl Eugen
James Almer Oct. 4, 2019, 1:06 a.m. UTC | #2
On 10/3/2019 9:26 PM, Carl Eugen Hoyos wrote:
> Am Fr., 4. Okt. 2019 um 01:59 Uhr schrieb James Almer <jamrial@gmail.com>:
>>
>> The rescaling can be done at muxing/encoding time, for formats that require it.
> 
> Doesn't this need a micro version bump?

No. The way these readers set the values is irrelevant. Writers will
either convert them to floats (Matroska, where rescaling is ultimately
unneeded), or effectively rescale them themselves
(h264/h465/vp9/av1/mp4, where it's required).

See 00fd38f1846a3c889b6ec645107eaea415b99840.

> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
James Almer Oct. 6, 2019, 1:39 a.m. UTC | #3
On 10/3/2019 8:52 PM, James Almer wrote:
> The rescaling can be done at muxing/encoding time, for formats that require it.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/matroskadec.c | 33 ++++++++++-----------------------
>  1 file changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 10c398856b..865f265c37 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2116,9 +2116,6 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
>      }
>  
>      if (has_mastering_primaries || has_mastering_luminance) {
> -        // Use similar rationals as other standards.
> -        const int chroma_den = 50000;
> -        const int luma_den = 10000;
>          AVMasteringDisplayMetadata *metadata =
>              (AVMasteringDisplayMetadata*) av_stream_new_side_data(
>                  st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
> @@ -2128,29 +2125,19 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
>          }
>          memset(metadata, 0, sizeof(AVMasteringDisplayMetadata));
>          if (has_mastering_primaries) {
> -            metadata->display_primaries[0][0] = av_make_q(
> -                round(mastering_meta->r_x * chroma_den), chroma_den);
> -            metadata->display_primaries[0][1] = av_make_q(
> -                round(mastering_meta->r_y * chroma_den), chroma_den);
> -            metadata->display_primaries[1][0] = av_make_q(
> -                round(mastering_meta->g_x * chroma_den), chroma_den);
> -            metadata->display_primaries[1][1] = av_make_q(
> -                round(mastering_meta->g_y * chroma_den), chroma_den);
> -            metadata->display_primaries[2][0] = av_make_q(
> -                round(mastering_meta->b_x * chroma_den), chroma_den);
> -            metadata->display_primaries[2][1] = av_make_q(
> -                round(mastering_meta->b_y * chroma_den), chroma_den);
> -            metadata->white_point[0] = av_make_q(
> -                round(mastering_meta->white_x * chroma_den), chroma_den);
> -            metadata->white_point[1] = av_make_q(
> -                round(mastering_meta->white_y * chroma_den), chroma_den);
> +            metadata->display_primaries[0][0] = av_d2q(mastering_meta->r_x, INT_MAX);
> +            metadata->display_primaries[0][1] = av_d2q(mastering_meta->r_y, INT_MAX);
> +            metadata->display_primaries[1][0] = av_d2q(mastering_meta->g_x, INT_MAX);
> +            metadata->display_primaries[1][1] = av_d2q(mastering_meta->g_y, INT_MAX);
> +            metadata->display_primaries[2][0] = av_d2q(mastering_meta->b_x, INT_MAX);
> +            metadata->display_primaries[2][1] = av_d2q(mastering_meta->b_y, INT_MAX);
> +            metadata->white_point[0] = av_d2q(mastering_meta->white_x, INT_MAX);
> +            metadata->white_point[1] = av_d2q(mastering_meta->white_y, INT_MAX);
>              metadata->has_primaries = 1;
>          }
>          if (has_mastering_luminance) {
> -            metadata->max_luminance = av_make_q(
> -                round(mastering_meta->max_luminance * luma_den), luma_den);
> -            metadata->min_luminance = av_make_q(
> -                round(mastering_meta->min_luminance * luma_den), luma_den);
> +            metadata->max_luminance = av_d2q(mastering_meta->max_luminance, INT_MAX);
> +            metadata->min_luminance = av_d2q(mastering_meta->min_luminance, INT_MAX);
>              metadata->has_luminance = 1;
>          }
>      }

Fixed the commit message (It's matroskadec, not matroskaenc) and pushed.
diff mbox

Patch

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 10c398856b..865f265c37 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2116,9 +2116,6 @@  static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
     }
 
     if (has_mastering_primaries || has_mastering_luminance) {
-        // Use similar rationals as other standards.
-        const int chroma_den = 50000;
-        const int luma_den = 10000;
         AVMasteringDisplayMetadata *metadata =
             (AVMasteringDisplayMetadata*) av_stream_new_side_data(
                 st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
@@ -2128,29 +2125,19 @@  static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
         }
         memset(metadata, 0, sizeof(AVMasteringDisplayMetadata));
         if (has_mastering_primaries) {
-            metadata->display_primaries[0][0] = av_make_q(
-                round(mastering_meta->r_x * chroma_den), chroma_den);
-            metadata->display_primaries[0][1] = av_make_q(
-                round(mastering_meta->r_y * chroma_den), chroma_den);
-            metadata->display_primaries[1][0] = av_make_q(
-                round(mastering_meta->g_x * chroma_den), chroma_den);
-            metadata->display_primaries[1][1] = av_make_q(
-                round(mastering_meta->g_y * chroma_den), chroma_den);
-            metadata->display_primaries[2][0] = av_make_q(
-                round(mastering_meta->b_x * chroma_den), chroma_den);
-            metadata->display_primaries[2][1] = av_make_q(
-                round(mastering_meta->b_y * chroma_den), chroma_den);
-            metadata->white_point[0] = av_make_q(
-                round(mastering_meta->white_x * chroma_den), chroma_den);
-            metadata->white_point[1] = av_make_q(
-                round(mastering_meta->white_y * chroma_den), chroma_den);
+            metadata->display_primaries[0][0] = av_d2q(mastering_meta->r_x, INT_MAX);
+            metadata->display_primaries[0][1] = av_d2q(mastering_meta->r_y, INT_MAX);
+            metadata->display_primaries[1][0] = av_d2q(mastering_meta->g_x, INT_MAX);
+            metadata->display_primaries[1][1] = av_d2q(mastering_meta->g_y, INT_MAX);
+            metadata->display_primaries[2][0] = av_d2q(mastering_meta->b_x, INT_MAX);
+            metadata->display_primaries[2][1] = av_d2q(mastering_meta->b_y, INT_MAX);
+            metadata->white_point[0] = av_d2q(mastering_meta->white_x, INT_MAX);
+            metadata->white_point[1] = av_d2q(mastering_meta->white_y, INT_MAX);
             metadata->has_primaries = 1;
         }
         if (has_mastering_luminance) {
-            metadata->max_luminance = av_make_q(
-                round(mastering_meta->max_luminance * luma_den), luma_den);
-            metadata->min_luminance = av_make_q(
-                round(mastering_meta->min_luminance * luma_den), luma_den);
+            metadata->max_luminance = av_d2q(mastering_meta->max_luminance, INT_MAX);
+            metadata->min_luminance = av_d2q(mastering_meta->min_luminance, INT_MAX);
             metadata->has_luminance = 1;
         }
     }