date:20240913

Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes

2024-09-13 Thread Nicolas Gaullier

>De : ffmpeg-devel  De la part de martin 
>schitter
>Envoyé : jeudi 12 septembre 2024 13:54
>On 12.09.24 13:14, Nicolas Gaullier wrote:
>> The message "Recognize and Ignore" does not make it clear what issue or 
>> grave defect is solved here.
>> I see in the code that fill items are currently recognized as dark metadata 
>> and ignored likewise, but I don't see any issue here.
>> Maybe could you comment a little bit about your intent ?
>
>While developing this DNxUncompressed code I always got lots of this "Dark key 
>..." log messages in the debug output. This kind of output wouldn't be a 
>surprise, if the key belongs to some rare and utterly irrelevant data >box. 
>but in this particular case the key stands for empty "fill" blocks, which are 
>frequently used in various places in MXF files. They are an elementary 
>building block of this container format.
>
>On the muxer side of ffmpegs MXF code 'fill' is known and used in many places, 
>but the demuxer doesn't recognize this element and just always prints these 
>warnings about something "unknown". That's highly irritating and >also 
>inefficient, because 'fill' is used quite frequently e.g. as place holder to 
>align frame data on 256byte boundaries etc.
>
>It's really trivial to fix and I don't see, why we should debate any longer 
>about this obvious flaw instead of just quickly solving the issue.
>
>But if you want, you can rewrite the wording to "Recognize 'fill' in MXF data 
>and suppress output" or whatever you like...

I am not against the idea of the patch: filler items should not be logged as 
dark metadata. I just wanted to check with you that it was indeed the only 
issue.
So it is not a big issue, and I find the patch confusing both because of the 
commit msg and code:
- the commit msg should not claim it "adds support for" filler items: mxf files 
with fillers are already supported/playable.
Maybe just a single line like "avformat/mxfdec: suppress verbose log for 
fillers" ?
- why not using a simple "if" on the av_log rather than inserting a new block 
of code ?

Thank you,
Nicolas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_av1: Fix encode fail since 9db68ed0

2024-09-13 Thread Lynne via ffmpeg-devel


On 13/09/2024 05:15, fei.w.wang-at-intel@ffmpeg.org wrote:

From: Fei Wang 

Signed-off-by: Fei Wang 
---
  libavcodec/vaapi_encode_av1.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vaapi_encode_av1.c b/libavcodec/vaapi_encode_av1.c
index 6d1be9fc07..1b350cd936 100644
--- a/libavcodec/vaapi_encode_av1.c
+++ b/libavcodec/vaapi_encode_av1.c
@@ -637,7 +637,7 @@ static int 
vaapi_encode_av1_init_picture_params(AVCodecContext *avctx,
  slot = ((VAAPIEncodeAV1Picture*)ref_pic->codec_priv)->slot;
  av_assert0(vpic->reference_frames[slot] == VA_INVALID_SURFACE);
  
-vpic->reference_frames[slot] = ((VAAPIEncodePicture *)ref_pic)->recon_surface;

+vpic->reference_frames[slot] = ((VAAPIEncodePicture 
*)ref_pic->priv)->recon_surface;
  }
  }
  


LGTM
thanks


OpenPGP_0xA2FEA5F03F034464.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/mxfdec.c: Recognize and Ignore MXF fill boxes

2024-09-13 Thread martin schitter

On 13.09.24 10:23, Nicolas Gaullier wrote:
I am not against the idea of the patch: filler items should not be logged as dark metadata.

Fine! -- that's the main point!

I find the patch confusing both because of the commit msg and code:
- the commit msg should not claim it "adds support for" filler items: mxf files
with fillers are already supported/playable.

It adds a small amount of support, because without adding this key entry
you can not identify and selective process 'fill' boxes.

Sure -- the files are already playable, and the processing is even
faster without any further handling of this specific kind of content,
but you will never be able to suppress the misleading verbose logging
without such a modification.

- why not using a simple "if" on the av_log rather than inserting a new block
of code ?

Somehow you always have to differentiate 'fill' boxes from other unknown
elements if you want to change the present behavior.

Sure -- it would be possible to change just the printed output in this
already present `av_log` call, but the necessary code would IMHO much
more "confusing":

a selective handling of a specific kind of entity in a branch of the
code flow, which is strictly dedicated to warn about "unknown" content.
That's simply a paradox, a developers joke or just something highly
"confusing".

45 matches

Mail list logo