Thanks for this careful review.
On 09.09.24 12:56, Anton Khirnov wrote:
Quoting Martin Schitter (2024-09-08 20:41:38)
diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c
new file mode 100644
index 0000000..455c374
--- /dev/null
+++ b/libavcodec/dnxucdec.c
@@ -0,0 +1,495 @@
+/*
+ * Avid DNxUncomressed / SMPTE RDD 50 demuxer
This says it's a demuxer, while it's implemented as a decoder.
Fixed.
I'm also wondering if this shouldn't be handled as demuxer exporting
raw video, at least in some of cases if not all.
When I started the work, I also thought, it will not require much more
than minimal modifications of MXF related raw data transport details,
but during reverse engineering the actually used dense bitpacking turned
out to be more complicated.
The codec section should fit rather well for this component, although
large parts of it could be also categorized just as bitstream filter.
+typedef struct DNxUcParseContext {
+ uint32_t fourcc_tag;
+ uint32_t width;
+ uint32_t height;
+ uint32_t nr_bytes;
+} DNxUcParseContext;
Parser should be in its own file, as it can be enabled or disabled
independently from the encoder.
O.k. that's possible.
Done.
Opening brace of a function body should be on its own line.
Fixed.
+ av_log(0, AV_LOG_ERROR, "can't read DNxUncompressed metadata.\n");
av_log() should always get a proper logging context, avctx in this case
Fixed.
+ *poutbuf_size = 0;
+ return buf_size;
+ }
+
+ pc->fourcc_tag = *(uint32_t*)(buf+24);
+ pc->width = *(uint32_t*)(buf+16);
+ pc->height = *(uint32_t*)(buf+20);
+ pc->nr_bytes = *(uint32_t*)(buf+29) - 8;
Use AV_RL32()
Done.
+ ret = ff_thread_get_buffer(avctx, frame, 0);
+ if (ret < 0)
+ return ret;
This call should not be duplicated in every handler.
Eliminated by some refactoring and an additional helper function.
+ if (avctx->width % 4){
+ av_log(0, AV_LOG_ERROR,
+ "Image width has to be dividable by 4 for 10bit RGB
DNxUncompressed!\n");
+ return AVERROR_EXIT;
+ }
These checks can be done once during init.
Yes -- A line width check for a multiple of 2 is now done only once for
all yuv4:2:2 compression variants. This is more standard compliant and
should work for all implemented format handlers.
+ ret = ff_thread_get_buffer(avctx, frame, 0);
+ if (ret < 0)
+ return ret;
+
+ lw = frame->width;
+
+ for(y = 0; y < frame->height; y++){
+ for(x = 0; x < frame->width; x++){
+ msp = pkt->data[y*3*(lw + lw/4) + 3*x];
Does anything guarantee the packet is large enough?
I added another packet size check, which should calculate the needed
size of data for this kind of very dense bit packed input.
I hope you like the chosen solutions.
Martin
_______________________________________________
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".
