diff mbox series

[FFmpeg-devel] avformat/movenc: remove unnecessary HDR metadata log messages

Message ID 20210209233151.11174-1-jeebjp@gmail.com
State Accepted
Commit 752f14ff942576dd8697320fb6ba055593a2de9d
Headers show
Series [FFmpeg-devel] avformat/movenc: remove unnecessary HDR metadata log messages | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Jan Ekström Feb. 9, 2021, 11:31 p.m. UTC 
These made sense before 3ebf449766d4b69a7774c4990fb24fe71c848a3e
when the creation of these boxes was specifically requested by the
user, but now they have become unnecessary as they are just like
many other boxes: If the input has the information required, they
are written, otherwise they are not.

They were moved to verbose verbosity level (which happens to be
the last level still relatively usable), and now appear either once
(normal MP4 muxing), or thrice (with the faststart flag set) in
any normal MP4 usage, without giving much useful information. Thus,
remove them in their current form.
---

For the record, I am not against having f.ex. the mdcv box writing function
warning in case the side data is there, but it lacks the required information.

But just always logging this stuff, even if the user didn't go anywhere near
streams with such information does not feel like something useful at this
level of verbosity (esp. with the faststart flag causing it to appear thrice).

Jan

---
 libavformat/movenc.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Anton Khirnov Feb. 10, 2021, 10:40 a.m. UTC | #1 
Quoting Jan Ekström (2021-02-10 00:31:51)
> These made sense before 3ebf449766d4b69a7774c4990fb24fe71c848a3e
> when the creation of these boxes was specifically requested by the
> user, but now they have become unnecessary as they are just like
> many other boxes: If the input has the information required, they
> are written, otherwise they are not.
> 
> They were moved to verbose verbosity level (which happens to be
> the last level still relatively usable), and now appear either once
> (normal MP4 muxing), or thrice (with the faststart flag set) in
> any normal MP4 usage, without giving much useful information. Thus,
> remove them in their current form.
> ---
> 
> For the record, I am not against having f.ex. the mdcv box writing function
> warning in case the side data is there, but it lacks the required information.
> 
> But just always logging this stuff, even if the user didn't go anywhere near
> streams with such information does not feel like something useful at this
> level of verbosity (esp. with the faststart flag causing it to appear thrice).
> 
> Jan
> 
> ---
>  libavformat/movenc.c | 2 --
>  1 file changed, 2 deletions(-)

av_log(NULL is evil, therefore I like this patch
Paul B Mahol Feb. 10, 2021, 10:42 a.m. UTC | #2 
lgtm
Jan Ekström Feb. 10, 2021, 7:52 p.m. UTC | #3 
On Wed, Feb 10, 2021 at 12:43 PM Paul B Mahol <onemda@gmail.com> wrote:
>
> lgtm

Cheers, applied as 752f14ff942576dd8697320fb6ba055593a2de9d .

Jan
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 372c04295d..62192e5c24 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2025,7 +2025,6 @@  static int mov_write_clli_tag(AVIOContext *pb, MOVTrack *track)
 
     side_data = av_stream_get_side_data(track->st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL, NULL);
     if (!side_data) {
-        av_log(NULL, AV_LOG_VERBOSE, "Not writing 'clli' atom. No content light level info.\n");
         return 0;
     }
     content_light_metadata = (const AVContentLightMetadata*)side_data;
@@ -2052,7 +2051,6 @@  static int mov_write_mdcv_tag(AVIOContext *pb, MOVTrack *track)
     side_data = av_stream_get_side_data(track->st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA, NULL);
     metadata = (const AVMasteringDisplayMetadata*)side_data;
     if (!metadata || !metadata->has_primaries || !metadata->has_luminance) {
-        av_log(NULL, AV_LOG_VERBOSE, "Not writing 'mdcv' atom. Missing mastering metadata.\n");
         return 0;
     }