* [FFmpeg-devel] [PATCH] avcodec/h2645_sei: loosen up min luminance requirements
@ 2024-05-25 11:36 Niklas Haas
2024-05-25 11:48 ` Kacper Michajlow
0 siblings, 1 reply; 2+ messages in thread
From: Niklas Haas @ 2024-05-25 11:36 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
The H.265 specification is quite clear on this case:
> When min_display_mastering_luminance is not in the range of 1 to
> 50000, the nominal maximum display luminance of the mastering display
> is unknown or unspecified or specified by other means not specified in
> this Specification.
And so the current code is correct in marking luminance data as invalid
if min luminance is set to 0. However, this breaks playback of at least
several real-world Blu-ray releases, for example La La Land, Planet of
the Apes, and quite possibly a lot more. These come with ostensibly
valid max_luminance tags (1000 nits), but min_luminance set to 0.
Loosen up this requirement by guarding it behind FF_COMPLIANCE_STRICT.
We still reject blatantly invalid metadata (wrong value range on
luminance, max set to 0, max below min, min above 50 nits etc.), so this
shouldn't cause any unintended regressions.
Fixes: https://github.com/mpv-player/mpv/issues/14177
---
libavcodec/h2645_sei.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
index 1deb76c765..7c83747cd0 100644
--- a/libavcodec/h2645_sei.c
+++ b/libavcodec/h2645_sei.c
@@ -619,11 +619,15 @@ static int h2645_sei_to_side_data(AVCodecContext *avctx, H2645SEI *sei,
metadata->min_luminance.num = sei->mastering_display.min_luminance;
metadata->min_luminance.den = luma_den;
- metadata->has_luminance &= sei->mastering_display.min_luminance >= 1 &&
- sei->mastering_display.min_luminance <= 50000 &&
+ metadata->has_luminance &= sei->mastering_display.min_luminance <= 50000 &&
sei->mastering_display.min_luminance <
sei->mastering_display.max_luminance;
+ /* Real (blu-ray) releases in the wild come with minimum luminance
+ * values of 0.000 cd/m2, so permit this edge case */
+ if (avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT)
+ metadata->has_luminance &= sei->mastering_display.min_luminance >= 1;
+
if (metadata->has_luminance || metadata->has_primaries)
av_log(avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
if (metadata->has_primaries) {
--
2.45.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/h2645_sei: loosen up min luminance requirements
2024-05-25 11:36 [FFmpeg-devel] [PATCH] avcodec/h2645_sei: loosen up min luminance requirements Niklas Haas
@ 2024-05-25 11:48 ` Kacper Michajlow
0 siblings, 0 replies; 2+ messages in thread
From: Kacper Michajlow @ 2024-05-25 11:48 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sat, 25 May 2024 at 13:36, Niklas Haas <ffmpeg@haasn.xyz> wrote:
>
> From: Niklas Haas <git@haasn.dev>
>
> The H.265 specification is quite clear on this case:
>
> > When min_display_mastering_luminance is not in the range of 1 to
> > 50000, the nominal maximum display luminance of the mastering display
> > is unknown or unspecified or specified by other means not specified in
> > this Specification.
>
> And so the current code is correct in marking luminance data as invalid
> if min luminance is set to 0. However, this breaks playback of at least
> several real-world Blu-ray releases, for example La La Land, Planet of
> the Apes, and quite possibly a lot more. These come with ostensibly
> valid max_luminance tags (1000 nits), but min_luminance set to 0.
>
> Loosen up this requirement by guarding it behind FF_COMPLIANCE_STRICT.
> We still reject blatantly invalid metadata (wrong value range on
> luminance, max set to 0, max below min, min above 50 nits etc.), so this
> shouldn't cause any unintended regressions.
>
> Fixes: https://github.com/mpv-player/mpv/issues/14177
> ---
> libavcodec/h2645_sei.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
> index 1deb76c765..7c83747cd0 100644
> --- a/libavcodec/h2645_sei.c
> +++ b/libavcodec/h2645_sei.c
> @@ -619,11 +619,15 @@ static int h2645_sei_to_side_data(AVCodecContext *avctx, H2645SEI *sei,
>
> metadata->min_luminance.num = sei->mastering_display.min_luminance;
> metadata->min_luminance.den = luma_den;
> - metadata->has_luminance &= sei->mastering_display.min_luminance >= 1 &&
> - sei->mastering_display.min_luminance <= 50000 &&
> + metadata->has_luminance &= sei->mastering_display.min_luminance <= 50000 &&
> sei->mastering_display.min_luminance <
> sei->mastering_display.max_luminance;
>
> + /* Real (blu-ray) releases in the wild come with minimum luminance
> + * values of 0.000 cd/m2, so permit this edge case */
> + if (avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT)
> + metadata->has_luminance &= sei->mastering_display.min_luminance >= 1;
> +
> if (metadata->has_luminance || metadata->has_primaries)
> av_log(avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
> if (metadata->has_primaries) {
> --
> 2.45.0
LGTM
- Kacper
_______________________________________________
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".
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-25 11:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-25 11:36 [FFmpeg-devel] [PATCH] avcodec/h2645_sei: loosen up min luminance requirements Niklas Haas
2024-05-25 11:48 ` Kacper Michajlow
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git