[FFmpeg-devel] [PATCH V4] avcodec/libvpxenc: add ROI-based encoding support for VP8/VP9 support
example command line to verify it:
./ffmpeg -i input.stream -vf addroi=0:0:iw/3:ih/3:-0.8 -c:v libvpx -b:v 2M
tmp.webm
Signed-off-by: Guo, Yejun
---
libavcodec/libvpxenc.c | 193 +
1 file changed, 193 insertions(+)
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index feb52ea..ed4 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -116,6 +116,9 @@ typedef struct VPxEncoderContext {
int tune_content;
int corpus_complexity;
int tpl_model;
+// If the driver does not support ROI then warn the first time we
+// encounter a frame with ROI side data.
+int roi_warned;
} VPxContext;
/** String mappings for enum vp8e_enc_control_id */
@@ -1057,6 +1060,188 @@ static int queue_frames(AVCodecContext *avctx, AVPacket
*pkt_out)
return size;
}
+static int set_roi_map(AVCodecContext *avctx, const AVFrameSideData *sd, int
frame_width, int frame_height,
+ vpx_roi_map_t *roi_map, int block_size, int segment_cnt)
+{
+/* range of vpx_roi_map_t.delta_q[i] is [-63, 63] */
+#define MAX_DELTA_Q 63
+
+const AVRegionOfInterest *roi = NULL;
+int nb_rois;
+uint32_t self_size;
+int segment_id;
+
+/* record the mapping from delta_q to "segment id + 1" in
segment_mapping[].
+ * the range of delta_q is [-MAX_DELTA_Q, MAX_DELTA_Q],
+ * and its corresponding array index is [0, 2 * MAX_DELTA_Q],
+ * and so the length of the mapping array is 2 * MAX_DELTA_Q + 1.
+ * "segment id + 1", so we can say there's no mapping if the value of
array element is zero.
+ */
+int segment_mapping[2 * MAX_DELTA_Q + 1] = { 0 };
+
+memset(roi_map, 0, sizeof(*roi_map));
+
+/* segment id 0 in roi_map is reserved for the areas not covered by
AVRegionOfInterest.
+ * segment id 0 in roi_map is also for the areas with
AVRegionOfInterest.qoffset near 0.
+ * (delta_q of segment id 0 is 0).
+ */
+segment_mapping[MAX_DELTA_Q] = 1;
+segment_id = 1;
+
+roi = (const AVRegionOfInterest*)sd->data;
+self_size = roi->self_size;
+if (!self_size || sd->size % self_size != 0) {
+av_log(avctx, AV_LOG_ERROR, "Invalid AVRegionOfInterest.self_size.\n");
+return AVERROR(EINVAL);
+}
+nb_rois = sd->size / self_size;
+
+/* This list must be iterated from zero because regions are
+ * defined in order of decreasing importance. So discard less
+ * important areas if they exceed the segment count.
+ */
+for (int i = 0; i < nb_rois; i++) {
+int delta_q;
+int mapping_index;
+
+roi = (const AVRegionOfInterest*)(sd->data + self_size * i);
+if (roi->qoffset.den == 0) {
+av_log(avctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must
not be zero.\n");
+return AVERROR(EINVAL);
+}
+
+delta_q = (int)(roi->qoffset.num * 1.0f / roi->qoffset.den *
MAX_DELTA_Q);
+delta_q = av_clip(delta_q, -MAX_DELTA_Q, MAX_DELTA_Q);
+
+mapping_index = delta_q + MAX_DELTA_Q;
+if (!segment_mapping[mapping_index]) {
+if (segment_id == segment_cnt) {
+av_log(avctx, AV_LOG_WARNING,
+ "ROI only supports %d segments (and segment 0 is
reserved for non-ROIs), skipping the left ones.\n",
+ segment_cnt);
+break;
+}
+
+segment_mapping[mapping_index] = segment_id + 1;
+roi_map->delta_q[segment_id] = delta_q;
+segment_id++;
+}
+}
+
+
+roi_map->rows = (frame_height + block_size - 1) / block_size;
+roi_map->cols = (frame_width + block_size - 1) / block_size;
+roi_map->roi_map = av_mallocz_array(roi_map->rows * roi_map->cols,
sizeof(*roi_map->roi_map));
+if (!roi_map->roi_map) {
+av_log(avctx, AV_LOG_ERROR, "roi_map alloc failed.\n");
+return AVERROR(ENOMEM);
+}
+
+/* This list must be iterated in reverse, so for the case that
+ * two regions overlapping, the more important area takes effect.
+ */
+for (int i = nb_rois - 1; i >= 0; i--) {
+int delta_q;
+int mapping_value;
+int starty, endy, startx, endx;
+
+roi = (const AVRegionOfInterest*)(sd->data + self_size * i);
+
+starty = av_clip(roi->top / block_size, 0, roi_map->rows);
+endy = av_clip((roi->bottom + block_size - 1) / block_size, 0,
roi_map->rows);
+startx = av_clip(roi->left / block_size, 0, roi_map->cols);
+endx = av_clip((roi->right + block_size - 1) / block_size, 0,
roi_map->cols);
+
+delta_q = (int)(roi->qoffset.num * 1.0f / roi->qoffset.den *
MAX_DELTA_Q);
+delta_q = av_clip(delta_q, -MAX_DELTA_Q, MAX_DELTA_Q);
+
+mapping_value = segment_mapping[delta_q + MAX_DELTA_Q];
+if (mapping_value) {
+for (int y = starty; y < endy; y++)
+for (int x = startx; x < endx; x++)
+
Re: [FFmpeg-devel] [PATCH] Change libaom default to crf=28.
On Tue, Aug 20, 2019 at 5:31 PM Elliott Karpilovsky wrote: > > I believe the documentation is out of date. I added some debug > statements and verified that variable was being used to pull CRF > values when AOM_Q mode is set. I have > https://aomedia-review.googlesource.com/c/aom/+/94104 out to update > the documentation. > Thanks for making the update to the docs. Note that inline replies are preferred to top-posting on this list. ___ 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".
[FFmpeg-devel] [PATCH v2] Add assembly support for -fsanitize=hwaddress tagged globals.
As of LLVM r368102, Clang will set a pointer tag in bits 56-63 of the address of a global when compiling with -fsanitize=hwaddress. This requires an adjustment to assembly code that takes the address of such globals: the code cannot use the regular R_AARCH64_ADR_PREL_PG_HI21 relocation to refer to the global, since the tag would take the address out of range. Instead, the code must use the non-checking (_NC) variant of the relocation (the link-time check is substituted by a runtime check). This change makes the necessary adjustment in the movrel macro, where it is needed when compiling with -fsanitize=hwaddress. Signed-off-by: Peter Collingbourne --- libavutil/aarch64/asm.S | 8 1 file changed, 8 insertions(+) diff --git a/libavutil/aarch64/asm.S b/libavutil/aarch64/asm.S index 5c329430fd..3ac2ba0d52 100644 --- a/libavutil/aarch64/asm.S +++ b/libavutil/aarch64/asm.S @@ -32,6 +32,10 @@ # define FUNC # #endif +#ifndef __has_feature +# define __has_feature(x) 0 +#endif + .macro function name, export=0, align=2 .macro endfunc ELF .size \name, . - \name @@ -94,7 +98,11 @@ ELF .size \name, . - \name add \rd, \rd, :lo12:\val+(\offset) .endif #elif CONFIG_PIC +# if __has_feature(hwaddress_sanitizer) +adrp\rd, :pg_hi21_nc:\val+(\offset) +# else adrp\rd, \val+(\offset) +# endif add \rd, \rd, :lo12:\val+(\offset) #else ldr \rd, =\val+\offset -- 2.23.0.187.g17f5b7556c-goog ___ 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/mxfenc: Allow more bitrates for NTSC IMX50
Hey guys,
> On Aug 19, 2019, at 9:54 AM, Thomas Mundt wrote:
>
> Am Fr., 16. Aug. 2019 um 23:31 Uhr schrieb Tomas Härdin > :
>
>> tor 2019-08-15 klockan 13:55 +0200 skrev Thomas Mundt:
>>> Am Do., 15. Aug. 2019 um 11:01 Uhr schrieb Tomas Härdin <
>> tjop...@acc.umu.se
:
ons 2019-08-14 klockan 22:18 +0200 skrev Thomas Mundt:
> Hi Tomas,
>
> Am Mi., 14. Aug. 2019 um 12:42 Uhr schrieb Tomas Härdin <
tjop...@acc.umu.se
>> :
>> tis 2019-08-13 klockan 22:03 +0200 skrev Thomas Mundt:
>>> Hi,
>>>
>>> attached patch fixes ticket #8077.
>>> Please comment.
>>
>> Probably OK, bitrates lower than 5000 are fine in D-10
>> according to
>> S356m.
>>
>>> } else if ((sc->video_bit_rate >= 4840) &&
>> (sc->video_bit_rate <=
>>> 5000) && (mxf->time_base.den != 25)) {
>>
>> You could drop the extra parentheses, else it should be fine.
>>
>
> New patch attached.
Looks OK. I'll push in a few days if no one else has any comments
>>>
>>> Thanks. Would you mind porting it to branches 4.1 and 4.2?
>>
>> I'm not quite sure what the process is for that. I have confirmed that
>> the problem exists in 4.1 and 4.2 and that your patch fixes it.
>>
>> I think we also might want to put a note somewhere in the documentation
>> how to make NTSC IMX50 files.
>>
Yeah, it’s been an issue for quite some time, s356m mentions:
"When used as a signal source for the type D-10 recording format, the bit
stream is carried by SDTI-CP, as defined in SMPTE 326M, using recommended
operating point bit rates as defined in this annex. Other bit rates may be
used. However, users are cautioned that other system design parameters within
the studio may not support all bit rates.
Table A.1 indicates recommended operating points to simplify studio operations
and to provide users with a tool to be used in designing systems."
Then specifies the exact value of the sequence_header bit_rate_value, 50mit/s
being “1E848h”, "To be used when compliant with EBU D84 and D85"
I don’t think it is a good idea to produce files with the wrong bit_rate value,
and I know for a fact that many file analyzers in use today will simply reject
the d-10 essence.
The current code is simply a hack, and IMHO the right solution is to put an
option to force the ratecontrol to behave as people want for NTSC D-10. I’ve
submitted a patch for that but Michael wanted it as an option.
—
Baptiste
___
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".
[FFmpeg-devel] [PATCH 4/4] tools/target_dec_fuzzer: Adjust max_pixels for IDCIN
Fixes: Timeout (128sec -> 6sec)
Fixes:
16568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IDCIN_fuzzer-5675004095627264
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
tools/target_dec_fuzzer.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index d83039417c..fd76553ec8 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -177,6 +177,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
case AV_CODEC_ID_GDV: maxpixels /= 256; break;
// Postprocessing in C
case AV_CODEC_ID_HNM4_VIDEO:maxpixels /= 128; break;
+// Unoptimized VLC reading and allows a small input to generate
gigantic output
+case AV_CODEC_ID_IDCIN: maxpixels /= 2048; break;
}
--
2.23.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/4] avformat/realtextdec: free queue on error
Fixes: memleak Fixes: 16277/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5696629440512000 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavformat/realtextdec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavformat/realtextdec.c b/libavformat/realtextdec.c index 204e557aa2..c2316da0ed 100644 --- a/libavformat/realtextdec.c +++ b/libavformat/realtextdec.c @@ -123,6 +123,8 @@ static int realtext_read_header(AVFormatContext *s) end: av_bprint_finalize(, NULL); +if (res < 0) +ff_subtitles_queue_clean(>q); return res; } -- 2.23.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/4] avcodec/iff: Check for overlap in cmap_read_palette()
Fixes: undefined mempcpy() use
Fixes:
16302/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IFF_ILBM_fuzzer-5678750575886336
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/iff.c | 4
1 file changed, 4 insertions(+)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index fc7bfad731..f612399daa 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -180,6 +180,10 @@ static int cmap_read_palette(AVCodecContext *avctx,
uint32_t *pal)
pal[i] = 0xFF00 | gray2rgb((i * 255) >>
avctx->bits_per_coded_sample);
}
if (s->masking == MASK_HAS_MASK) {
+if ((1 << avctx->bits_per_coded_sample) < count) {
+avpriv_request_sample(avctx, "overlaping mask");
+return AVERROR_PATCHWELCOME;
+}
memcpy(pal + (1 << avctx->bits_per_coded_sample), pal, count * 4);
for (i = 0; i < count; i++)
pal[i] &= 0xFF;
--
2.23.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/4] avformat/vividas: Check av_xiphlacing() return value before use
Fixes: out of array access
Fixes:
16277/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5696629440512000
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavformat/vividas.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/libavformat/vividas.c b/libavformat/vividas.c
index 0c33ca2da8..645e322b6e 100644
--- a/libavformat/vividas.c
+++ b/libavformat/vividas.c
@@ -392,8 +392,14 @@ static int track_header(VividasDemuxContext *viv,
AVFormatContext *s, uint8_t *
p = st->codecpar->extradata;
p[0] = 2;
-for (j = 0; j < num_data - 1; j++)
-offset += av_xiphlacing([offset], data_len[j]);
+for (j = 0; j < num_data - 1; j++) {
+unsigned delta = av_xiphlacing([offset], data_len[j]);
+if (delta > data_len[j]) {
+av_free(pb);
+return AVERROR_INVALIDDATA;
+}
+offset += delta;
+}
for (j = 0; j < num_data; j++) {
int ret = avio_read(pb, [offset], data_len[j]);
--
2.23.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat: Fix probing on some JPEGs
On Mon, Aug 19, 2019 at 7:22 PM Carl Eugen Hoyos wrote: > > This score would mean that mjpeg can never be detected. > I suspect you have to reduce one of the demuxers to "- 1". > > Thanks Carl. Attached patch to reduce mpeg probe by -1, which also fixes the issue. Alternatively I could bump both jpeg and mjpeg? mpegps_probe looks like it has some heuristics to come up with its scores, which may make it brittle. -- Nikolas Bowe | SWE | nb...@google.com | 408-565-5137 From ab42e2041ec1469f43bccbf8c3e06084bbb7985a Mon Sep 17 00:00:00 2001 From: Nikolas Bowe Date: Thu, 8 Aug 2019 15:32:51 -0700 Subject: [PATCH] avformat: Fix probing on some JPEGs Fixes "Invalid data found when processing input" on some JPEGs. Some large extensionless JPEGs can get probe score collisions with mpeg eg $ ffprobe -loglevel trace /tmp/foo [NULL @ 0x55c130ab04c0] Opening '/tmp/foo' for reading [file @ 0x55c130ab0f40] Setting default whitelist 'file,crypto' Probing jpeg_pipe score:6 size:2048 Probing jpeg_pipe score:6 size:4096 Probing jpeg_pipe score:6 size:8192 Probing jpeg_pipe score:6 size:16384 Probing jpeg_pipe score:25 size:32768 Probing jpeg_pipe score:25 size:65536 Probing jpeg_pipe score:25 size:131072 Probing jpeg_pipe score:25 size:262144 Probing jpeg_pipe score:25 size:524288 Probing mpeg score:25 size:1048576 Probing jpeg_pipe score:25 size:1048576 [AVIOContext @ 0x55c130ab9300] Statistics: 1048576 bytes read, 0 seeks /tmp/foo: Invalid data found when processing input This patch fixes this by reducing probe score for mpeg --- libavformat/mpeg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c index 3205f209e6..7a7de54894 100644 --- a/libavformat/mpeg.c +++ b/libavformat/mpeg.c @@ -100,7 +100,7 @@ static int mpegps_probe(const AVProbeData *p) } if (vid + audio > invalid + 1) /* invalid VDR files nd short PES streams */ -score = AVPROBE_SCORE_EXTENSION / 2; +score = AVPROBE_SCORE_EXTENSION / 2 - 1; // 1 less than jpeg in SOS // av_log(NULL, AV_LOG_ERROR, "vid:%d aud:%d sys:%d pspack:%d invalid:%d size:%d \n", //vid, audio, sys, pspack, invalid, p->buf_size); -- 2.23.0.rc1.153.gdeed80330f-goog ___ 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] Change libaom default to crf=32.
On Wed, Aug 21, 2019 at 12:18 PM Elliott Karpilovsky wrote: > > From: elliottk > > Current default is 256kbps, which produces inconsistent > results (too high for low-res, too low for hi-res). > Use CRF instead, which will adapt. > --- > libavcodec/libaomenc.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > lgtm. I'll apply this soon if there aren't any other comments. libvpx could probably use a similar update. ___ 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 1/4] avformat/realtextdec: free queue on error
On 8/21/2019 7:18 PM, Michael Niedermayer wrote: > Fixes: memleak > Fixes: > 16277/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5696629440512000 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/realtextdec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/realtextdec.c b/libavformat/realtextdec.c > index 204e557aa2..c2316da0ed 100644 > --- a/libavformat/realtextdec.c > +++ b/libavformat/realtextdec.c > @@ -123,6 +123,8 @@ static int realtext_read_header(AVFormatContext *s) > > end: > av_bprint_finalize(, NULL); > +if (res < 0) > +ff_subtitles_queue_clean(>q); LGTM > return res; > } > > ___ 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] Change libaom default to crf=28.
It appears that CRF=32 is a better default, since the scale is
different from other codecs. I will start a new thread.
On Tue, Aug 20, 2019 at 5:31 PM Elliott Karpilovsky wrote:
>
> I believe the documentation is out of date. I added some debug
> statements and verified that variable was being used to pull CRF
> values when AOM_Q mode is set. I have
> https://aomedia-review.googlesource.com/c/aom/+/94104 out to update
> the documentation.
>
>
> On Fri, Aug 16, 2019 at 10:59 AM James Zern
> wrote:
> >
> > Hi,
> >
> > On Thu, Aug 15, 2019 at 1:22 PM elliottk
> > wrote:
> > >
> > > Current default is 256kbps, which produces inconsistent
> > > results (too high for low-res, too low for hi-res).
> > > Use CRF instead, which will adapt.
> > > ---
> > > libavcodec/libaomenc.c | 9 +
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > > index 9b4fb3b4eb..621e897672 100644
> > > --- a/libavcodec/libaomenc.c
> > > +++ b/libavcodec/libaomenc.c
> > > @@ -575,10 +575,11 @@ static av_cold int aom_init(AVCodecContext *avctx,
> > > if (enccfg.rc_end_usage == AOM_CQ) {
> > > enccfg.rc_target_bitrate = 100;
> > > } else {
> > > -avctx->bit_rate = enccfg.rc_target_bitrate * 1000;
> > > +enccfg.rc_end_usage = AOM_Q;
> >
> > Unless the docs are out of date this should be AOM_CQ level since crf
> > is mapped to that control [1].
> >
> > > +ctx->crf = 28;
> > > av_log(avctx, AV_LOG_WARNING,
> > > - "Neither bitrate nor constrained quality specified,
> > > using default bitrate of %dkbit/sec\n",
> > > - enccfg.rc_target_bitrate);
> > > + "Neither bitrate nor constrained quality specified,
> > > using default CRF of %d\n",
> > > + ctx->crf);
> > > }
> > > }
> > >
> >
> > [1] https://aomedia.googlesource.com/aom/+/refs/heads/master/aom/aomcx.h#221
> > "For this value to be used aom_codec_enc_cfg_t::rc_end_usage must be
> > set to #AOM_CQ."
> > ___
> > 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".
___
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] lavf: Add DICOM demuxer, lavc: Add DICOM decoder
On Thu, Aug 22, 2019 at 00:28:28 +0530, Shivam wrote: > >> + dicom->height = *(uint16_t *)bytes; > > does this work on big endian ? > > maybe you where looking for AV_RL16() > > Big endian DICOM files are retired and no longer supported by the standard. What Michael means: What happens if you use this to decode a (little endian) DICOM file on big endian hardware? In other words: "dicom->height = *(uint16_t *)bytes" will map the bytes into the big endian int in the wrong order, resulting in an incorrect integer value. AV_RL16() (e.g.) handles this properly for you. @Michael, do you have some short instructions on how to set up a big endian system like your MIPS+qemu? Thanks. Moritz ___ 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] [REQUEST] avcodec/scpr: revert d70276921348
On Tue, 20 Aug 2019, Carl Eugen Hoyos wrote: Am Di., 20. Aug. 2019 um 14:48 Uhr schrieb Paul B Mahol : I kindly ask that following commit: d702769213487923c0fb0abe4b61f4d9ebddb88b I still believe what the patch does is a very good idea and a revert would hurt FFmpeg. If the patch is kept then probably a workaround is needed here as well for an issue like #7880, as it affects all cases where duplicated frames are dropped. Regards, Marton ___ 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] Add assembly support for -fsanitize=hwaddress tagged globals.
On Thu, Aug 15, 2019 at 11:00 AM Peter Collingbourne wrote: > > As of LLVM r368102, Clang will set a pointer tag in bits 56-63 of the > address of a global when compiling with -fsanitize=hwaddress. This requires > an adjustment to assembly code that takes the address of such globals: the > code cannot use the regular R_AARCH64_ADR_PREL_PG_HI21 relocation to refer > to the global, since the tag would take the address out of range. Instead, > the code must use the non-checking (_NC) variant of the relocation (the > link-time check is substituted by a runtime check). > > This change makes the necessary adjustment in the movrel macro, where it is > needed when compiling with -fsanitize=hwaddress. It came to my attention that this patch was sent without a Signed-off-by line. Will send a v2 with the Signed-off-by. Peter ___ 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] avdevice/decklink: adjust for timecode lag
On Tue, 20 Aug 2019, Devin Heitmueller wrote: A couple of follow-up Qs: Is auto-detection available for all Decklink devices? No, but AFAIK it is for all devices which support SDI. Generally it's the older analog capture devices which don't support it. For those for which it is available, are there any edge cases in which it sets inaccurate mode? I don't trust the existing detection code enough to use it in production. It often fails to detect and thus ffmpeg will exit at startup. Can this be fixed by simply increasing the timeout from 1 sec to 2 seconds? Also, there are cases where it will misdetect 1080i59 as 1080p30 depending on the card. It's been on my TODO list for a while to make that code more robust (I believe I know what most of the issues are), but it hasn't been critical for any of my use cases. Hmm, interesting... When you say the issue is card-dependant, does this mean card _model_ dependant or that the issue can affect one card and not the other with of the same model/fw? Thanks, Marton ___ 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".
[FFmpeg-devel] [PATCH] Change libaom default to crf=32.
From: elliottk
Current default is 256kbps, which produces inconsistent
results (too high for low-res, too low for hi-res).
Use CRF instead, which will adapt.
---
libavcodec/libaomenc.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 9b4fb3b4eb..7f47707a09 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -575,10 +575,11 @@ static av_cold int aom_init(AVCodecContext *avctx,
if (enccfg.rc_end_usage == AOM_CQ) {
enccfg.rc_target_bitrate = 100;
} else {
-avctx->bit_rate = enccfg.rc_target_bitrate * 1000;
+enccfg.rc_end_usage = AOM_Q;
+ctx->crf = 32;
av_log(avctx, AV_LOG_WARNING,
- "Neither bitrate nor constrained quality specified, using
default bitrate of %dkbit/sec\n",
- enccfg.rc_target_bitrate);
+ "Neither bitrate nor constrained quality specified, using
default CRF of %d\n",
+ ctx->crf);
}
}
@@ -1091,7 +1092,7 @@ static const AVOption options[] = {
};
static const AVCodecDefault defaults[] = {
-{ "b", "256*1000" },
+{ "b", "0" },
{ "qmin", "-1" },
{ "qmax", "-1" },
{ "g","-1" },
--
2.23.0.rc1.153.gdeed80330f-goog
___
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 1/3] avformat/mxfdec: do not ignore bad size errors
On Sun, 18 Aug 2019, Tomas Härdin wrote:
lör 2019-08-17 klockan 21:41 +0200 skrev Marton Balint:
The return value was unintentionally lost after
00a2652df3bf25a27d174cc67ed508b5317cb115.
Signed-off-by: Marton Balint
---
libavformat/mxfdec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index bb72fb9841..397f820b3f 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -3508,8 +3508,8 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket
*pkt)
} else {
if ((size = next_ofs - pos) <= 0) {
av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size);
-ret = AVERROR_INVALIDDATA;
-goto skip;
+mxf->current_klv_data = (KLVPacket){{0}};
+return AVERROR_INVALIDDATA;
Should maybe ask for a sample. Else looks OK
I don't think this can happen with any of the valid files, so the text
saying that ffmpeg is missing a feature probably won't be true. It is a
lot more likely that the user has a broken file if this error is shown.
So I'd rather keep it as is.
Regards,
Marton
___
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".
[FFmpeg-devel] [PATCH 1/3] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 71 ++
libavutil/encode_info.h | 110
libavutil/frame.c | 1 +
libavutil/frame.h | 7 +++
5 files changed, 191 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..91e43fce63
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,71 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, unsigned nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(unsigned nb_blocks)
+{
+// Check for possible size_t overflow
+if (nb_blocks - 1 > (SIZE_MAX - sizeof(AVEncodeInfoFrame)) /
sizeof(AVEncodeInfoBlock))
+return NULL;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) - sizeof(AVEncodeInfoBlock) +
+ FFMAX(1, nb_blocks) * sizeof(AVEncodeInfoBlock);
+
+AVEncodeInfoFrame *ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, unsigned
nb_blocks)
+{
+// Check for size_t overflow
+if (nb_blocks - 1 > (SIZE_MAX - sizeof(AVEncodeInfoFrame)) /
sizeof(AVEncodeInfoBlock))
+return NULL;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) - sizeof(AVEncodeInfoBlock) +
+ FFMAX(1, nb_blocks) * sizeof(AVEncodeInfoBlock);
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git
[FFmpeg-devel] [PATCH] avcodec/omx: add support for -force_key_frames
From: Aman Gupta
Signed-off-by: Aman Gupta
---
libavcodec/omx.c | 20
1 file changed, 20 insertions(+)
diff --git a/libavcodec/omx.c b/libavcodec/omx.c
index a1e5a46a54..8c722b573c 100644
--- a/libavcodec/omx.c
+++ b/libavcodec/omx.c
@@ -802,6 +802,26 @@ static int omx_encode_frame(AVCodecContext *avctx,
AVPacket *pkt,
// Convert the timestamps to microseconds; some encoders can ignore
// the framerate and do VFR bit allocation based on timestamps.
buffer->nTimeStamp = to_omx_ticks(av_rescale_q(frame->pts,
avctx->time_base, AV_TIME_BASE_Q));
+if (frame->pict_type == AV_PICTURE_TYPE_I) {
+#if CONFIG_OMX_RPI
+OMX_CONFIG_BOOLEANTYPE config = {0, };
+INIT_STRUCT(config);
+config.bEnabled = OMX_TRUE;
+err = OMX_SetConfig(s->handle,
OMX_IndexConfigBrcmVideoRequestIFrame, );
+if (err != OMX_ErrorNone) {
+av_log(avctx, AV_LOG_ERROR, "OMX_SetConfig(RequestIFrame)
failed: %x\n", err);
+}
+#else
+OMX_CONFIG_INTRAREFRESHVOPTYPE config = {0, };
+INIT_STRUCT(config);
+config.nPortIndex = s->out_port;
+config.IntraRefreshVOP = OMX_TRUE;
+err = OMX_SetConfig(s->handle,
OMX_IndexConfigVideoIntraVOPRefresh, );
+if (err != OMX_ErrorNone) {
+av_log(avctx, AV_LOG_ERROR, "OMX_SetConfig(IntraVOPRefresh)
failed: %x\n", err);
+}
+#endif
+}
err = OMX_EmptyThisBuffer(s->handle, buffer);
if (err != OMX_ErrorNone) {
append_buffer(>input_mutex, >input_cond,
>num_free_in_buffers, s->free_in_buffers, buffer);
--
2.20.1
___
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] lavf: Add DICOM demuxer, lavc: Add DICOM decoder
Forwarded Message
Subject: Re: [FFmpeg-devel] [PATCH] lavf: Add DICOM demuxer, lavc: Add
DICOM decoder
Date: Thu, 22 Aug 2019 00:27:55 +0530
From: Shivam
To: Michael Niedermayer
On 8/21/19 2:00 AM, Michael Niedermayer wrote:
On Tue, Aug 20, 2019 at 08:53:43PM +0530, Shivam wrote:
Sorry, for my previous mail, i forgot to attach the patch.
The patch contains support for Dicom files. The below features are
supported
yet:-
Uncompressed DICOM files using any of the Implicit and Explicit VR
formats.
Multiframe files are also supported.
To extract the metadata or info about the procedure, I have added an
option
"-metadata." (The tags present in Dicom dictionary would be demuxed).
I have also uploaded DICOM samples here.
https://1drv.ms/u/s!AosJ9_SrP4Tsh2WoPfQAI8cSsqUs?e=ZMd5fR
Please review,
Thank you,
Shivam Goyal
Changelog | 1
libavcodec/Makefile | 1
libavcodec/allcodecs.c | 1
libavcodec/avcodec.h | 1
libavcodec/codec_desc.c | 7
libavcodec/dicom.c | 189
libavcodec/version.h | 2
libavformat/Makefile | 1
libavformat/allformats.c | 1
libavformat/dicom.h | 108 +++
libavformat/dicomdec.c | 594 ++
libavformat/dicomdict.c | 716
+++
libavformat/version.h | 2
13 files changed, 1622 insertions(+), 2 deletions(-)
d47b7ad6a9f16ce4e29a67a99800183f9056062d add_dicom.patch
From cd7ebe834278b29b0429b3f5b377154490ee159f Mon Sep 17 00:00:00 2001
From: Shivam Goyal <1998.goyal.shi...@gmail.com>
Date: Tue, 20 Aug 2019 20:03:02 +0530
Subject: [PATCH] lavc: add DICOM decoder, lavf: add DICOM demuxer
---
Changelog | 1 +
libavcodec/Makefile | 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/avcodec.h | 1 +
libavcodec/codec_desc.c | 7 +
libavcodec/dicom.c | 189 +++
libavcodec/version.h | 2 +-
libavformat/Makefile | 1 +
libavformat/allformats.c | 1 +
libavformat/dicom.h | 108 ++
libavformat/dicomdec.c | 594
libavformat/dicomdict.c | 716 +++
libavformat/version.h | 2 +-
libavformat and libavcodec changes should be in seperate patches
13 files changed, 1622 insertions(+), 2 deletions(-)
create mode 100644 libavcodec/dicom.c
create mode 100644 libavformat/dicom.h
create mode 100644 libavformat/dicomdec.c
create mode 100644 libavformat/dicomdict.c
diff --git a/Changelog b/Changelog
index c1f1237770..e04c3aa5f6 100644
--- a/Changelog
+++ b/Changelog
@@ -4,6 +4,7 @@ releases are sorted from youngest to oldest.
version :
- Intel QSV-accelerated MJPEG decoding
- Intel QSV-accelerated VP9 decoding
+- DICOM decoder and demxer
version 4.2:
- tpad filter
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index e49188357b..799da8aef7 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -263,6 +263,7 @@ OBJS-$(CONFIG_DCA_DECODER) += dcadec.o dca.o
dcadata.o dcahuff.o \
OBJS-$(CONFIG_DCA_ENCODER) += dcaenc.o dca.o dcadata.o dcahuff.o \
dcaadpcm.o
OBJS-$(CONFIG_DDS_DECODER) += dds.o
+OBJS-$(CONFIG_DICOM_DECODER) += dicom.o
OBJS-$(CONFIG_DIRAC_DECODER) += diracdec.o dirac.o diracdsp.o
diractab.o \
dirac_arith.o dirac_dwt.o dirac_vlc.o
OBJS-$(CONFIG_DFA_DECODER) += dfa.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 22985325e0..02a1afa7e8 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -83,6 +83,7 @@ extern AVCodec ff_cscd_decoder;
extern AVCodec ff_cyuv_decoder;
extern AVCodec ff_dds_decoder;
extern AVCodec ff_dfa_decoder;
+extern AVCodec ff_dicom_decoder;
extern AVCodec ff_dirac_decoder;
extern AVCodec ff_dnxhd_encoder;
extern AVCodec ff_dnxhd_decoder;
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index d234271c5b..756e168c75 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -410,6 +410,7 @@ enum AVCodecID {
AV_CODEC_ID_SCREENPRESSO,
AV_CODEC_ID_RSCC,
AV_CODEC_ID_AVS2,
+ AV_CODEC_ID_DICOM,
AV_CODEC_ID_Y41P = 0x8000,
AV_CODEC_ID_AVRP,
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 4d033c20ff..ae9abdb2ba 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1403,6 +1403,13 @@ static const AVCodecDescriptor
codec_descriptors[] = {
.long_name = NULL_IF_CONFIG_SMALL("AVS2-P2/IEEE1857.4"),
.props = AV_CODEC_PROP_LOSSY,
},
+ {
+ .id = AV_CODEC_ID_DICOM,
+ .type = AVMEDIA_TYPE_VIDEO,
+ .name = "dicom",
+ .long_name = NULL_IF_CONFIG_SMALL("DICOM (Digital Imaging and
Communications in Medicine)"),
+ .props = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,
+ },
{
.id = AV_CODEC_ID_Y41P,
.type = AVMEDIA_TYPE_VIDEO,
diff --git a/libavcodec/dicom.c b/libavcodec/dicom.c
new file mode 100644
index 00..eaa378d944
--- /dev/null
+++ b/libavcodec/dicom.c
@@ -0,0 +1,189 @@
+/*
+ * DICOM decoder
+ * Copyright (c) 2019 Shivam Goyal
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as
Re: [FFmpeg-devel] [PATCH] lavf: Add DICOM demuxer, lavc: Add DICOM decoder
On 8/21/19 7:27 PM, Moritz Barsnick wrote:
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
Please review,
(Untested, just visual code inspection.)
+- DICOM decoder and demxer
Typo -> demuxer. Also, when splitting the commits, also split the
changes to the Changelog. (Can still be one line eventually.)
+.props = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,
Possibly also AV_CODEC_PROP_INTRA_ONLY? (PNG doesn't have that either,
but other still image formats do.) Is DICOM a still image format, or
does it have multiple images and a sense of I-frames?
+static int extract_ed(AVCodecContext *avctx)
The return value is never used anywhere.
+int ed_s = avctx->extradata_size;
Feel free to name the variable with something containing "size", makes
the code somewhat easier to understand.
+static uint8_t apply_transfm(int64_t val, int64_t bitmask, int pix_pad,
^ I see no reason to save two letters in this
name. ;-)
+static int decode_mono( AVCodecContext *avctx,
+const uint8_t *buf,
+AVFrame *p)
^ spurious space
+switch (bitsalloc) {
+case 8:
ffmpeg uses the same indentation level for "case" as for "switch".
+for (i = 0; i < size; i++) {
+if (pixrep)
+pix = (int8_t)bytestream_get_byte();
+else
+pix = (uint8_t)bytestream_get_byte();
+out[i] = pix;
+}
What is this doing? Is the cast to uint8_t an implicit "abs()"?
Could it just be:
pix = pixrep ? bytestream_get_byte() :
FFABS(bytestream_get_byte());
out[i] = ...
Or in a somewhat different style:
uintXY_t val = bytestream_get_byte();
pix = pixrep ? byte : FFABS(byte);
out[i] = ...
+default:
+av_log(avctx, AV_LOG_ERROR, "Bits allocated %d not supported\n",
bitsalloc);
+return AVERROR_INVALIDDATA;
avctx->bits_per_coded_sample is a constant per file, right?
This "default" could be avoided if avctx->bits_per_coded_sample were
checked in init(), not in decode_frame().
+av_log(avctx, AV_LOG_ERROR, "Required buffer size is %d but recieved only
%d\n", req_buf_size, buf_size);
typo: received
+void* bytes;
void *bytes
+char* desc; // description (from dicom dictionary)
char *desc;
+const uint8_t *d = p->buf;
+
+if (d[DICOM_PREAMBLE_SIZE] == 'D' &&
+d[DICOM_PREAMBLE_SIZE + 1] == 'I' &&
+d[DICOM_PREAMBLE_SIZE + 2] == 'C' &&
+d[DICOM_PREAMBLE_SIZE + 3] == 'M') {
+return AVPROBE_SCORE_MAX;
Would:
if (!strncmp(p->buf, "DICM", 4)
also work? Seems much simpler to me. (Also skipping the intermediate
"d" variable.)
+if (de->bytes)
+av_free(de->bytes);
+if (de->desc)
+av_free(de->desc);
As Michael said, av_free() includes the NULL checks already.
Additionally, I believe the use of av_freep() is preferred for these
pointers. (Provokes a segfault on use after free, instead of "silently"
writing into / reading from that memory.)
+// detects transfer syntax
+static TransferSyntax get_transfer_sytax (const char *ts) {
^ typo: syntax
Could you also please not name the variable "ts", as that already has
too many other meanings. ;-) (Use e.g. "syntax".)
+static void set_dec_extradata(AVFormatContext *s, AVStream *st) {
+DICOMContext *dicom = s->priv_data;
+uint8_t *edp;
+int size = 20 + AV_INPUT_BUFFER_PADDING_SIZE;
+
+st->codecpar->extradata = (uint8_t *)av_malloc(size);
+edp = st->codecpar->extradata;
+bytestream_put_le32(>codecpar->extradata, dicom->interpret);
+bytestream_put_le32(>codecpar->extradata, dicom->pixrep);
+bytestream_put_le32(>codecpar->extradata, dicom->pixpad);
+bytestream_put_le32(>codecpar->extradata, dicom->slope);
+bytestream_put_le32(>codecpar->extradata, dicom->intcpt);
+st->codecpar->extradata = edp;
+st->codecpar->extradata_size = size;
+}
I'm not sure you're doing anything with edp here. Did you mean to use:
bytestream_put_le32(, dicom->interpret);
?
+sprintf(key, "(%04x,%04x) %s", de->GroupNumber, de->ElementNumber,
de->desc);
As Michael said, this can overflow "key". *Always* use snprintf.
+switch(de->VR) {
+case AT:
Indentation.
+static int set_multiframe_data(AVFormatContext *s, AVStream* st, DataElement
*de)
+{
+DICOMContext *dicom = s->priv_data;
+
+if (de->GroupNumber != MF_GR_NB)
+return 0;
+
+switch (de->ElementNumber) {
+case 0x1063: // frame time
+dicom->delay = conv_DS(de);
+dicom->duration = dicom->delay * dicom->nb_frames;
+break;
+}
+return 0;
+}
Always returns 0.
Is this a switch/case, so that it can be expanded in the future?
+av_log(s, AV_LOG_WARNING,"Data
Re: [FFmpeg-devel] [CALL] New FFmpeg developers meeting
On 20.08.2019, at 10:57, Paul B Mahol wrote: > Because of current overall toxic situation in FFmpeg, meeting will not be > held until situation improves considerably. It's a bit strange to read a such a opposite statement without any reason given for the change of mind. Also while I can see that it can and likely will feel quite uncomfortable, an actual meeting usually is one of the more effective ways to actually improve the situation. Though admittedly it doesn't need to be anything "formally" organized. ___ 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 1/3] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-19):
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 70 +
> libavutil/encode_info.h | 110
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7 +++
> 5 files changed, 190 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..348f7cda29
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,70 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +// To prevent overflow, assumes max number = 1px blocks for 8k video.
> +#define AV_ENCODE_INFO_MAX_BLOCKS 33177600
Urgh, a magical number hardcoded. And it stills overflows if
sizeof(block)>129, which it was when you had the reserved field.
Define this in terms of SIZE_MAX, sizeof(info) and sizeof(block).
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, unsigned int
> nb_blocks) {
Here and everywhere, "unsigned int" can be shortened into "unsigned". I
think it is better, because experienced programmers may see "int foo"
and not notice the "unsigned" in front immediately.
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(unsigned int nb_blocks)
> +{
> +if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
> +return NULL;
> +
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> + sizeof(AVEncodeInfoBlock)*(!nb_blocks ? 0 : nb_blocks - 1);
As I told you, the formula can be simplified as:
sizeof(info) - sizeof(block) + FFMAX(1, n) * sizeof(block)
> +AVEncodeInfoFrame *ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, unsigned
> int nb_blocks)
> +{
> +if (nb_blocks >
Re: [FFmpeg-devel] [PATCH v2 1/3] avfilter: add v360 filter
On Tue, Aug 20, 2019 at 7:04 PM James Almer wrote: > On 8/20/2019 5:29 AM, Paul B Mahol wrote: > > On Mon, Aug 19, 2019 at 11:31 PM James Almer wrote: > > > >> On 8/13/2019 10:14 PM, Eugene Lyapustin wrote: > >>> Signed-off-by: Eugene Lyapustin > >>> --- > >>> doc/filters.texi | 137 +++ > >>> libavfilter/Makefile |1 + > >>> libavfilter/allfilters.c |1 + > >>> libavfilter/vf_v360.c| 1847 ++ > >>> 4 files changed, 1986 insertions(+) > >>> create mode 100644 libavfilter/vf_v360.c > >>> > >>> diff --git a/doc/filters.texi b/doc/filters.texi > >> > >> Shouldn't this make use of the AVSphericalMapping frame side data if > >> available? > >> > > > > It is extremely limited API. > > It can surely be extended if required. > Yes, it is required first. > > > And due lavfi limitations it can only clear current side data and set > new one. > > Isn't that enough? This filter converts from one projection to another. > Replacing the side data after said conversion to reflect the changes > would be the expected behavior. > Yes. But no guessing of input format is currently possible IMHO. > > > > > > >> ___ > >> 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". > > ___ > > 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". > > > > ___ > 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". ___ 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".
[FFmpeg-devel] Legal status of MLP decoder/encoder + MLP DVD-Audio headers
Hi there, I'm Fabrice Nicol, the developer of the free software dvda-author, which creates DVD-Audio discs (http://dvd-audio.sourceforge.net/) 1. With the help of Jai Luthra, the MLP encoder maintainer, I've been implementing DVD-Audio authoring for MLP files using libavcodec. This will be an important step forward, as MLP makes it possible to implement full 5.1 High res audio on DVDs, a long-awaited feature in the free world (plus there is no longer any maintained commercial software that offers it either, the only two ones that did it have be retrieved from the market). I was wondering whether the ffmpeg team has investigated the potential legal issues with the MLP decoder/encoder, as it was originally created under proprietary licensing terms. In particular, I'm wary that uploading to Sourceforge servers hosted in the US may involve potential risks in this respect. Do you think it necessary to refrain from posting on SF and sticking to European-hosted servers, or is this just unecessary? 2. MLP on DVD-Audio is just MLP files with MPEG-like headers (64 or 43 bytes long). I've been able to infer and implement 90 % of these headers but there are 4 remaining bytes at the end of headers with which I'm stuck. They strongly differ from the reference for LPCM DVD-Audio. LPCM values are sample rate/size/downmix. MLP values cannot be that. I was wondering if the ffmpeg specialists who worked on MLP or possibly MPEG could have an idea. Issue is referenced in lines 577-582 of this blob : https://sourceforge.net/p/dvd-audio/dev/ci/master/tree/src/ats.c Best, Fabrice Nicol ___ 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 0/3] Make avio_enum_protocols const correct
Am Mi., 21. Aug. 2019 um 11:13 Uhr schrieb Andreas Rheinhardt : > > Hello, > > this goal of this patchset is making avio_enum_protocols const correct. > It currently ignores the distinction between const URLProtocol * > const * and const URLProtocol ** in the line p = p ? p + 1 : url_protocols; > (where p is of the latter type and url_protocols is of the former (after > the array-to-pointer conversion has taken place)). As a consequence, the > users of this function will have pointers to non-const pointing to > something that is actually const. > Fixing this requires changing the function's signature and this will > only be possible at the next major version bump. I believe this was changed to const without a major bump (but this has to be checked) and in the general case, it shouldn't be a problem. 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] tools/target_dec_fuzzer: use refcounted packets
tis 2019-08-20 klockan 21:05 -0300 skrev James Almer:
> Should reduce date copying considerably.
>
> Signed-off-by: James Almer
> ---
> Fixed a stupid mistake when checking the return value for av_new_packet().
> Still untested.
Works great for me. Should make fuzzing faster overall, better use of
computing resources imo
> @@ -186,6 +144,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
> size) {
> error("Failed memory allocation");
>
> ctx->max_pixels = maxpixels_per_frame; //To reduce false positive OOM
> and hangs
> +ctx->refcounted_frames = 1;
Could maybe have a comment that this is also to reduce false positives,
or that we want to focus on the new API rather than the old one
> @@ -240,7 +199,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
> size) {
> if (data + sizeof(fuzz_tag) > end)
> data = end;
>
> -FDBPrepare(, , last, data - last);
> +res = av_new_packet(, data - last);
> +if (res < 0)
> +error("Failed memory allocation");
> +memcpy(parsepkt.data, last, data - last);
Is there some way to avoid this copy?
/Tomas
___
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/g729dec: Support decoding ACELP.KELVIN
On Wed, Aug 21, 2019 at 11:12 AM Carl Eugen Hoyos wrote: > Am Mi., 21. Aug. 2019 um 10:54 Uhr schrieb Paul B Mahol >: > > > > On Wed, Aug 21, 2019 at 10:44 AM Carl Eugen Hoyos > > wrote: > > > > > Am Mi., 21. Aug. 2019 um 10:40 Uhr schrieb Paul B Mahol < > one...@gmail.com > > > > > > > > The configure line is not needed. Where is Makefile change? > > > > > > This line makes no sense. > > > > Nope, you do not make sense. > > I hope you agree it is difficult to "make sense" in an answer to > something that makes no sense. > Please consult doctors. > > > Configure line is completely bollocks. > > Works fine here. > Remove it, its is not relevant. > > > Makefile change is mandatory and in your case it is missing completely. > > Mandatory? > I seem to remember a time when I tried to keep the whole magic in the > Makefiles. Somebody said "no, the maintenance burden is too high, > we do some of the magic in configure and some in the Makefile to make > our lifes easier". What is it now? Should we move all the changes back > in the Makefiles? > Are you trolling now? libavcodec/Makefile change is missing. Also you need to use #ifdef for decoders in g729 file as user may enable only one of decoders at time. > > 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-requ...@ffmpeg.org with subject "unsubscribe". ___ 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] Legal status of MLP decoder/encoder + MLP DVD-Audio headers
On Wed, Aug 21, 2019 at 10:29 AM Carl Eugen Hoyos wrote: > Am Mi., 21. Aug. 2019 um 10:01 Uhr schrieb fabrice nicol < > fabrni...@gmail.com>: > > > I was wondering whether the ffmpeg team has investigated the potential > > legal issues with the MLP decoder/encoder, as it was originally created > > under proprietary licensing terms. > > To the best of our knowledge, the mlp decoder was written by Ian Caulfield, > the mlp encoder by Jai Luthra, both were released under the terms of the > LGPL. > mlp encoder was not written by Jai Luthra at all. Why are you constantly spreading misinformation? > > > In particular, I'm wary that uploading to Sourceforge servers hosted in > > the US may involve potential risks in this respect. Do you think it > > necessary to refrain from posting on SF and sticking to > > European-hosted servers, or is this just unecessary? > > Only your intellectual-property lawyer can answer this question, > afaik nobody on this mailing list is an intellectual property lawyer. > > 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-requ...@ffmpeg.org with subject "unsubscribe". ___ 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".
[FFmpeg-devel] [PATCH 3/3] avformat/wtvdec: Forward errors when reading packet
wtvfile_read_packet did not abide by the requirements of a function
destined to read a packet: If it did not read anything, it returned
zero, which currently leads to a warning in read_packet_wrapper in
aviobuf.c. Said warning will be an av_assert2 as soon as
FF_API_OLD_AVIO_EOF_0 is zero (probably the next major version bump).
So instead forward the error code from the underlying protocol; if this
was never ever called or returned zero, return AVERROR_EOF as
read_packet_wrapper currently does.
This error/assert is triggered in the wtv-demux FATE test.
Signed-off-by: Andreas Rheinhardt
---
I am not sure what the right approach is if nread and n are both zero.
Is it actually allowed for the buf_size argument to be zero? (In this
case, this scenario would happen.)
libavformat/wtvdec.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 706e8ca38d..56c54f6aba 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -71,7 +71,7 @@ static int wtvfile_read_packet(void *opaque, uint8_t *buf,
int buf_size)
{
WtvFile *wf = opaque;
AVIOContext *pb = wf->pb_filesystem;
-int nread = 0;
+int nread = 0, n = 0;
if (wf->error || pb->error)
return -1;
@@ -80,7 +80,6 @@ static int wtvfile_read_packet(void *opaque, uint8_t *buf,
int buf_size)
buf_size = FFMIN(buf_size, wf->length - wf->position);
while(nread < buf_size) {
-int n;
int remaining_in_sector = (1 << wf->sector_bits) - (wf->position & ((1
<< wf->sector_bits) - 1));
int read_request= FFMIN(buf_size - nread, remaining_in_sector);
@@ -100,7 +99,7 @@ static int wtvfile_read_packet(void *opaque, uint8_t *buf,
int buf_size)
}
}
}
-return nread;
+return nread ? nread : n ? n : AVERROR_EOF;
}
/**
--
2.21.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] fate: Don't use depreceated keepside option
The tests for concat use this option which is scheduled for removal and
does nothing any more. So remove it; otherwise, these tests would fail
at the next major version bump.
Signed-off-by: Andreas Rheinhardt
---
tests/fate-run.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 2f1991da52..09f80ffdc2 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -471,10 +471,10 @@ concat(){
awk "{gsub(/%SRCFILE%/, \"$sample\"); print}" $template > $concatfile
if [ "$mode" = "md5" ]; then
-run ffprobe${PROGSUF}${EXECSUF} -bitexact -show_streams -show_packets
-v 0 -fflags keepside -safe 0 $extra_args $concatfile | tr -d '\r' > $packetfile
+run ffprobe${PROGSUF}${EXECSUF} -bitexact -show_streams -show_packets
-v 0 -safe 0 $extra_args $concatfile | tr -d '\r' > $packetfile
do_md5sum $packetfile
else
-run ffprobe${PROGSUF}${EXECSUF} -bitexact -show_streams -show_packets
-v 0 -of compact=p=0:nk=1 -fflags keepside -safe 0 $extra_args $concatfile
+run ffprobe${PROGSUF}${EXECSUF} -bitexact -show_streams -show_packets
-v 0 -of compact=p=0:nk=1 -safe 0 $extra_args $concatfile
fi
}
--
2.21.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] lavformat: Prepare to make avio_enum_protocols const correct
Using avio_enum_protocols works as follows: One initializes a pointer to
void and gives avio_enum_protocols the address of said pointer as
argument; the pointer will be updated to point to a member of the
url_protocols array. Now the address of the pointer can be reused for
another call to avio_enum_protocols.
Said array consists of constant pointers (to constant URLProtocols),
but the user now has a pointer to non-const to it; of course it was always
intended that the user is not allowed to modify what the pointer points
to and this has been enforced by hiding the real type of the underlying
object. But it is better to use a const void ** as parameter to enforce
this. This way avio_enum_protocols can be implemented without resorting
to casting a const away or ignoring constness as is done currently.
Given that this amounts to an ABI and API break, this can only be done
at the next major version bump; as usual, the break is currently hidden
behind an appropriate #if.
It was also necessary to change the type of a pointer used in
avio_enum_protocols. This makes the line that is not const correct move
as long as the old function signature is used. With the new signature,
avio_enum_protocols will be const correct.
This change will eventually force changes in their callers, e.g. to
show_protocols in fftools/cmdutils. (This function contains all currently
existing calls to avio_enum_protocols in FFmpeg's codebase. It hasn't
been touched in this commit.)
Signed-off-by: Andreas Rheinhardt
---
libavformat/avio.h | 4
libavformat/protocols.c | 6 +-
libavformat/version.h | 3 +++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 9141642e75..e067ea8985 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -805,7 +805,11 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer);
*
* @return A static string containing the name of current protocol or NULL
*/
+#if FF_API_NONCONST_ENUM_PROTOCOLS
const char *avio_enum_protocols(void **opaque, int output);
+#else
+const char *avio_enum_protocols(const void **opaque, int output);
+#endif
/**
* Pause and resume playing - only meaningful if using a network streaming
diff --git a/libavformat/protocols.c b/libavformat/protocols.c
index ad95659795..c722f9a897 100644
--- a/libavformat/protocols.c
+++ b/libavformat/protocols.c
@@ -91,9 +91,13 @@ const AVClass *ff_urlcontext_child_class_next(const AVClass
*prev)
}
+#if FF_API_NONCONST_ENUM_PROTOCOLS
const char *avio_enum_protocols(void **opaque, int output)
+#else
+const char *avio_enum_protocols(const void **opaque, int output)
+#endif
{
-const URLProtocol **p = *opaque;
+const URLProtocol * const *p = *opaque;
p = p ? p + 1 : url_protocols;
*opaque = p;
diff --git a/libavformat/version.h b/libavformat/version.h
index 9814db8633..b0b9264382 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -106,6 +106,9 @@
#ifndef FF_API_AVIOFORMAT
#define FF_API_AVIOFORMAT (LIBAVFORMAT_VERSION_MAJOR < 59)
#endif
+#ifndef FF_API_NONCONST_ENUM_PROTOCOLS
+#define FF_API_NONCONST_ENUM_PROTOCOLS (LIBAVFORMAT_VERSION_MAJOR < 59)
+#endif
#ifndef FF_API_R_FRAME_RATE
--
2.21.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] lavformat: Prepare to make avio_enum_protocols const correct
ons 2019-08-21 klockan 11:04 +0200 skrev Andreas Rheinhardt: > Using avio_enum_protocols works as follows: One initializes a pointer to > void and gives avio_enum_protocols the address of said pointer as > argument; the pointer will be updated to point to a member of the > url_protocols array. Now the address of the pointer can be reused for > another call to avio_enum_protocols. > Said array consists of constant pointers (to constant URLProtocols), > but the user now has a pointer to non-const to it; of course it was always > intended that the user is not allowed to modify what the pointer points > to and this has been enforced by hiding the real type of the underlying > object. But it is better to use a const void ** as parameter to enforce > this. This way avio_enum_protocols can be implemented without resorting > to casting a const away or ignoring constness as is done currently. > > Given that this amounts to an ABI and API break, this can only be done > at the next major version bump; as usual, the break is currently hidden > behind an appropriate #if. I'm fairly sure this is only an API break. C ABI doesn't care about constness. But also: > @@ -805,7 +805,11 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t > **pbuffer); > * > * @return A static string containing the name of current protocol or NULL > */ > +#if FF_API_NONCONST_ENUM_PROTOCOLS > const char *avio_enum_protocols(void **opaque, int output); > +#else > +const char *avio_enum_protocols(const void **opaque, int output); > +#endif This should still be perfectly compatible with all user code since adding const is fine.. /Tomas ___ 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/g729dec: Support decoding ACELP.KELVIN
On Wed, Aug 21, 2019, at 11:35, Paul B Mahol wrote: > > > > > The configure line is not needed. Where is Makefile change? > > > > > > > > This line makes no sense. > > > > > > Nope, you do not make sense. > > > > I hope you agree it is difficult to "make sense" in an answer to > > something that makes no sense. > > Please consult doctors. Those kind of remarks are not OK. -- Jean-Baptiste Kempf - President +33 672 704 734 ___ 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] Legal status of MLP decoder/encoder + MLP DVD-Audio headers
Am Mi., 21. Aug. 2019 um 10:01 Uhr schrieb fabrice nicol : > I was wondering whether the ffmpeg team has investigated the potential > legal issues with the MLP decoder/encoder, as it was originally created > under proprietary licensing terms. To the best of our knowledge, the mlp decoder was written by Ian Caulfield, the mlp encoder by Jai Luthra, both were released under the terms of the LGPL. > In particular, I'm wary that uploading to Sourceforge servers hosted in > the US may involve potential risks in this respect. Do you think it > necessary to refrain from posting on SF and sticking to > European-hosted servers, or is this just unecessary? Only your intellectual-property lawyer can answer this question, afaik nobody on this mailing list is an intellectual property lawyer. 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH]lavc/g729dec: Support decoding ACELP.KELVIN
The configure line is not needed. Where is Makefile change? Changlelog one does not apply. On Tue, Aug 20, 2019 at 7:25 PM Carl Eugen Hoyos wrote: > Am Di., 20. Aug. 2019 um 10:30 Uhr schrieb Paul B Mahol >: > > > > Do not use full caps name for short name. > > New patch attached. > > Thank you, 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-requ...@ffmpeg.org with subject "unsubscribe". ___ 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/g729dec: Support decoding ACELP.KELVIN
Am Mi., 21. Aug. 2019 um 10:40 Uhr schrieb Paul B Mahol : > > The configure line is not needed. Where is Makefile change? This line makes no sense. 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/3] Make avio_enum_protocols const correct
Carl Eugen Hoyos: > Am Mi., 21. Aug. 2019 um 11:13 Uhr schrieb Andreas Rheinhardt > : >> >> Hello, >> >> this goal of this patchset is making avio_enum_protocols const correct. >> It currently ignores the distinction between const URLProtocol * >> const * and const URLProtocol ** in the line p = p ? p + 1 : url_protocols; >> (where p is of the latter type and url_protocols is of the former (after >> the array-to-pointer conversion has taken place)). As a consequence, the >> users of this function will have pointers to non-const pointing to >> something that is actually const. > >> Fixing this requires changing the function's signature and this will >> only be possible at the next major version bump. > > I believe this was changed to const without a major bump (but this > has to be checked) and in the general case, it shouldn't be a problem. > > Carl Eugen I presume your "this" refers to the url_protocols array (and to the other lists)? If so, this was changed in fd9212f2 without a bump, but this does not mean that we don't need a bump here: The C standard contains the clause "pointers to qualified or unqualified versions of compatible types shall have the same representation and alignment requirements" which (if I am not mistaken) means that one can change the type of a function parameter from void * to const void *. But in this case the change is from void ** to const void ** and so the aforementioned clause doesn't apply at all. - Andreas ___ 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/g729dec: Support decoding ACELP.KELVIN
On Wed, Aug 21, 2019 at 10:44 AM Carl Eugen Hoyos wrote: > Am Mi., 21. Aug. 2019 um 10:40 Uhr schrieb Paul B Mahol >: > > > > The configure line is not needed. Where is Makefile change? > > This line makes no sense. > Nope, you do not make sense. Configure line is completely bollocks. Makefile change is mandatory and in your case it is missing completely. > > 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-requ...@ffmpeg.org with subject "unsubscribe". ___ 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 1/2] avcodec/vp5/6/8: use vpX_rac_is_end()
On Tue, Aug 20, 2019 at 11:51:48AM +0200, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/vp5.c | 2 +-
> libavcodec/vp6.c | 2 +-
> libavcodec/vp8.c | 8
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/vp5.c b/libavcodec/vp5.c
> index 49988b8b76..0fca282918 100644
> --- a/libavcodec/vp5.c
> +++ b/libavcodec/vp5.c
> @@ -183,7 +183,7 @@ static int vp5_parse_coeff(VP56Context *s)
> int b, i, cg, idx, ctx, ctx_last;
> int pt = 0;/* plane type (0 for Y, 1 for U or V) */
>
> -if (c->end <= c->buffer && c->bits >= 0) {
> +if (vpX_rac_is_end(c)) {
> av_log(s->avctx, AV_LOG_ERROR, "End of AC stream reached in
> vp5_parse_coeff\n");
> return AVERROR_INVALIDDATA;
> }
> diff --git a/libavcodec/vp6.c b/libavcodec/vp6.c
> index 977fcb7076..e5dec19f50 100644
> --- a/libavcodec/vp6.c
> +++ b/libavcodec/vp6.c
> @@ -473,7 +473,7 @@ static int vp6_parse_coeff(VP56Context *s)
> int b, i, cg, idx, ctx;
> int pt = 0;/* plane type (0 for Y, 1 for U or V) */
>
> -if (c->end <= c->buffer && c->bits >= 0) {
> +if (vpX_rac_is_end(c)) {
> av_log(s->avctx, AV_LOG_ERROR, "End of AC stream reached in
> vp6_parse_coeff\n");
> return AVERROR_INVALIDDATA;
> }
> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> index efc62aabaf..eb51d1f3c9 100644
> --- a/libavcodec/vp8.c
> +++ b/libavcodec/vp8.c
> @@ -658,7 +658,7 @@ static int vp7_decode_frame_header(VP8Context *s, const
> uint8_t *buf, int buf_si
> s->fade_present = vp8_rac_get(c);
> }
>
> -if (c->end <= c->buffer && c->bits >= 0)
> +if (vpX_rac_is_end(c))
> return AVERROR_INVALIDDATA;
> /* E. Fading information for previous frame */
> if (s->fade_present && vp8_rac_get(c)) {
> @@ -693,7 +693,7 @@ static int vp7_decode_frame_header(VP8Context *s, const
> uint8_t *buf, int buf_si
> vp78_update_pred16x16_pred8x8_mvc_probabilities(s, VP7_MVC_SIZE);
> }
>
> -if (c->end <= c->buffer && c->bits >= 0)
> +if (vpX_rac_is_end(c))
> return AVERROR_INVALIDDATA;
>
> if ((ret = vp7_fade_frame(s, alpha, beta)) < 0)
> @@ -2375,7 +2375,7 @@ static av_always_inline int
> decode_mb_row_no_filter(AVCodecContext *avctx, void
> curframe->tf.f->data[2] + 8 * mb_y * s->uvlinesize
> };
>
> -if (c->end <= c->buffer && c->bits >= 0)
> +if (vpX_rac_is_end(c))
> return AVERROR_INVALIDDATA;
>
> if (mb_y == 0)
> @@ -2406,7 +2406,7 @@ static av_always_inline int
> decode_mb_row_no_filter(AVCodecContext *avctx, void
> td->mv_bounds.mv_max.x = ((s->mb_width - 1) << 6) + MARGIN;
>
> for (mb_x = 0; mb_x < s->mb_width; mb_x++, mb_xy++, mb++) {
> -if (c->end <= c->buffer && c->bits >= 0)
> +if (vpX_rac_is_end(c))
> return AVERROR_INVALIDDATA;
> // Wait for previous thread to read mb_x+2, and reach mb_y-1.
> if (prev_td != td) {
approve.
-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
signature.asc
Description: PGP 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] lavf: Add DICOM demuxer, lavc: Add DICOM decoder
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
> Please review,
(Untested, just visual code inspection.)
> +- DICOM decoder and demxer
Typo -> demuxer. Also, when splitting the commits, also split the
changes to the Changelog. (Can still be one line eventually.)
> +.props = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,
Possibly also AV_CODEC_PROP_INTRA_ONLY? (PNG doesn't have that either,
but other still image formats do.) Is DICOM a still image format, or
does it have multiple images and a sense of I-frames?
> +static int extract_ed(AVCodecContext *avctx)
The return value is never used anywhere.
> +int ed_s = avctx->extradata_size;
Feel free to name the variable with something containing "size", makes
the code somewhat easier to understand.
> +static uint8_t apply_transfm(int64_t val, int64_t bitmask, int pix_pad,
^ I see no reason to save two letters in this
name. ;-)
> +static int decode_mono( AVCodecContext *avctx,
> +const uint8_t *buf,
> +AVFrame *p)
^ spurious space
> +switch (bitsalloc) {
> +case 8:
ffmpeg uses the same indentation level for "case" as for "switch".
> +for (i = 0; i < size; i++) {
> +if (pixrep)
> +pix = (int8_t)bytestream_get_byte();
> +else
> +pix = (uint8_t)bytestream_get_byte();
> +out[i] = pix;
> +}
What is this doing? Is the cast to uint8_t an implicit "abs()"?
Could it just be:
pix = pixrep ? bytestream_get_byte() :
FFABS(bytestream_get_byte());
out[i] = ...
Or in a somewhat different style:
uintXY_t val = bytestream_get_byte();
pix = pixrep ? byte : FFABS(byte);
out[i] = ...
> +default:
> +av_log(avctx, AV_LOG_ERROR, "Bits allocated %d not supported\n",
> bitsalloc);
> +return AVERROR_INVALIDDATA;
avctx->bits_per_coded_sample is a constant per file, right?
This "default" could be avoided if avctx->bits_per_coded_sample were
checked in init(), not in decode_frame().
> +av_log(avctx, AV_LOG_ERROR, "Required buffer size is %d but recieved
> only %d\n", req_buf_size, buf_size);
typo: received
> +void* bytes;
void *bytes
> +char* desc; // description (from dicom dictionary)
char *desc;
> +const uint8_t *d = p->buf;
> +
> +if (d[DICOM_PREAMBLE_SIZE] == 'D' &&
> +d[DICOM_PREAMBLE_SIZE + 1] == 'I' &&
> +d[DICOM_PREAMBLE_SIZE + 2] == 'C' &&
> +d[DICOM_PREAMBLE_SIZE + 3] == 'M') {
> +return AVPROBE_SCORE_MAX;
Would:
if (!strncmp(p->buf, "DICM", 4)
also work? Seems much simpler to me. (Also skipping the intermediate
"d" variable.)
> +if (de->bytes)
> +av_free(de->bytes);
> +if (de->desc)
> +av_free(de->desc);
As Michael said, av_free() includes the NULL checks already.
Additionally, I believe the use of av_freep() is preferred for these
pointers. (Provokes a segfault on use after free, instead of "silently"
writing into / reading from that memory.)
> +// detects transfer syntax
> +static TransferSyntax get_transfer_sytax (const char *ts) {
^ typo: syntax
Could you also please not name the variable "ts", as that already has
too many other meanings. ;-) (Use e.g. "syntax".)
> +static void set_dec_extradata(AVFormatContext *s, AVStream *st) {
> +DICOMContext *dicom = s->priv_data;
> +uint8_t *edp;
> +int size = 20 + AV_INPUT_BUFFER_PADDING_SIZE;
> +
> +st->codecpar->extradata = (uint8_t *)av_malloc(size);
> +edp = st->codecpar->extradata;
> +bytestream_put_le32(>codecpar->extradata, dicom->interpret);
> +bytestream_put_le32(>codecpar->extradata, dicom->pixrep);
> +bytestream_put_le32(>codecpar->extradata, dicom->pixpad);
> +bytestream_put_le32(>codecpar->extradata, dicom->slope);
> +bytestream_put_le32(>codecpar->extradata, dicom->intcpt);
> +st->codecpar->extradata = edp;
> +st->codecpar->extradata_size = size;
> +}
I'm not sure you're doing anything with edp here. Did you mean to use:
bytestream_put_le32(, dicom->interpret);
?
> +sprintf(key, "(%04x,%04x) %s", de->GroupNumber, de->ElementNumber,
> de->desc);
As Michael said, this can overflow "key". *Always* use snprintf.
> +switch(de->VR) {
> +case AT:
Indentation.
> +static int set_multiframe_data(AVFormatContext *s, AVStream* st, DataElement
> *de)
> +{
> +DICOMContext *dicom = s->priv_data;
> +
> +if (de->GroupNumber != MF_GR_NB)
> +return 0;
> +
> +switch (de->ElementNumber) {
> +case 0x1063: // frame time
> +dicom->delay = conv_DS(de);
> +dicom->duration = dicom->delay * dicom->nb_frames;
> +break;
> +}
> +return 0;
> +}
Always returns 0.
Is this a switch/case, so that it can be
[FFmpeg-devel] [PATCH 0/3] Make avio_enum_protocols const correct
Hello, this goal of this patchset is making avio_enum_protocols const correct. It currently ignores the distinction between const URLProtocol * const * and const URLProtocol ** in the line p = p ? p + 1 : url_protocols; (where p is of the latter type and url_protocols is of the former (after the array-to-pointer conversion has taken place)). As a consequence, the users of this function will have pointers to non-const pointing to something that is actually const. Fixing this requires changing the function's signature and this will only be possible at the next major version bump. Given that the FF_API defines are not part of the public API and following Carl-Eugen's lead in constifying the AVOutputFormat pointer I did not change show_protocols in fftools/cmdutils.c (this function contains all the callers of avio_enum_protocols in FFmpeg). Also included are two patches found via running FATE with the major version bumped locally. ([1] has also been found that way.) - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248144.html Andreas Rheinhardt (3): lavformat: Prepare to make avio_enum_protocols const correct fate: Don't use depreceated keepside option avformat/wtvdec: Forward errors when reading packet libavformat/avio.h | 4 libavformat/protocols.c | 6 +- libavformat/version.h | 3 +++ libavformat/wtvdec.c| 5 ++--- tests/fate-run.sh | 4 ++-- 5 files changed, 16 insertions(+), 6 deletions(-) -- 2.21.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH]lavc/g729dec: Support decoding ACELP.KELVIN
Am Mi., 21. Aug. 2019 um 10:54 Uhr schrieb Paul B Mahol : > > On Wed, Aug 21, 2019 at 10:44 AM Carl Eugen Hoyos > wrote: > > > Am Mi., 21. Aug. 2019 um 10:40 Uhr schrieb Paul B Mahol > > > > > The configure line is not needed. Where is Makefile change? > > > > This line makes no sense. > > Nope, you do not make sense. I hope you agree it is difficult to "make sense" in an answer to something that makes no sense. > Configure line is completely bollocks. Works fine here. > Makefile change is mandatory and in your case it is missing completely. Mandatory? I seem to remember a time when I tried to keep the whole magic in the Makefiles. Somebody said "no, the maintenance burden is too high, we do some of the magic in configure and some in the Makefile to make our lifes easier". What is it now? Should we move all the changes back in the Makefiles? 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] lavformat: Prepare to make avio_enum_protocols const correct
Tomas Härdin: > ons 2019-08-21 klockan 11:04 +0200 skrev Andreas Rheinhardt: >> Using avio_enum_protocols works as follows: One initializes a pointer to >> void and gives avio_enum_protocols the address of said pointer as >> argument; the pointer will be updated to point to a member of the >> url_protocols array. Now the address of the pointer can be reused for >> another call to avio_enum_protocols. >> Said array consists of constant pointers (to constant URLProtocols), >> but the user now has a pointer to non-const to it; of course it was always >> intended that the user is not allowed to modify what the pointer points >> to and this has been enforced by hiding the real type of the underlying >> object. But it is better to use a const void ** as parameter to enforce >> this. This way avio_enum_protocols can be implemented without resorting >> to casting a const away or ignoring constness as is done currently. >> >> Given that this amounts to an ABI and API break, this can only be done >> at the next major version bump; as usual, the break is currently hidden >> behind an appropriate #if. > > I'm fairly sure this is only an API break. C ABI doesn't care about > constness. But also: > >> @@ -805,7 +805,11 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer); >> * >> * @return A static string containing the name of current protocol or NULL >> */ >> +#if FF_API_NONCONST_ENUM_PROTOCOLS >> const char *avio_enum_protocols(void **opaque, int output); >> +#else >> +const char *avio_enum_protocols(const void **opaque, int output); >> +#endif > > This should still be perfectly compatible with all user code since > adding const is fine.. > No. void* can be safely and automatically converted to const void*; and the conversion from void** to void * const * is fine, too, but the conversion from void ** to const void ** is not safe for the reason already mentioned. I'll explain it once more. Imagine avio_enum_protocols already used a const void ** parameter and were const-correct and we called the function in the following way: void *opaque = NULL; avio_enum_protocols(, 0); opaque now points to something const (namely the first element of the url_protocols array, an array of const pointers (to constant URLProtocols)), but opaque is not a pointer to something const, i.e. a violation of the const system has happened. Therefore one needs an explicit cast for this unsafe conversion; or the compiler complains. It is of course easy to fix this: Simply declare opaque as const void *. As mentioned already, the caller has no business modifying what this pointer points to anyway. - Andreas ___ 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".
[FFmpeg-devel] [PATCH 2/2] doc/examples/decode_video: add input file format information for usage
Signed-off-by: Steven Liu
---
doc/examples/decode_video.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/examples/decode_video.c b/doc/examples/decode_video.c
index 5a9d43f689..169188a4b9 100644
--- a/doc/examples/decode_video.c
+++ b/doc/examples/decode_video.c
@@ -95,7 +95,8 @@ int main(int argc, char **argv)
AVPacket *pkt;
if (argc <= 2) {
-fprintf(stderr, "Usage: %s \n", argv[0]);
+fprintf(stderr, "Usage: %s \n"
+"And check your input file is encoded by mpeg1video
please.\n", argv[0]);
exit(0);
}
filename= argv[1];
--
2.17.2 (Apple Git-113)
___
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".
[FFmpeg-devel] [PATCH 1/2] doc/examples/decode_audio: print message about how to play the output file
Signed-off-by: Steven Liu
---
doc/examples/decode_audio.c | 51 +
1 file changed, 51 insertions(+)
diff --git a/doc/examples/decode_audio.c b/doc/examples/decode_audio.c
index 19dcafd2c8..23d0c9bf50 100644
--- a/doc/examples/decode_audio.c
+++ b/doc/examples/decode_audio.c
@@ -39,6 +39,35 @@
#define AUDIO_INBUF_SIZE 20480
#define AUDIO_REFILL_THRESH 4096
+static int get_format_from_sample_fmt(const char **fmt,
+ enum AVSampleFormat sample_fmt)
+{
+int i;
+struct sample_fmt_entry {
+enum AVSampleFormat sample_fmt; const char *fmt_be, *fmt_le;
+} sample_fmt_entries[] = {
+{ AV_SAMPLE_FMT_U8, "u8","u8"},
+{ AV_SAMPLE_FMT_S16, "s16be", "s16le" },
+{ AV_SAMPLE_FMT_S32, "s32be", "s32le" },
+{ AV_SAMPLE_FMT_FLT, "f32be", "f32le" },
+{ AV_SAMPLE_FMT_DBL, "f64be", "f64le" },
+};
+*fmt = NULL;
+
+for (i = 0; i < FF_ARRAY_ELEMS(sample_fmt_entries); i++) {
+struct sample_fmt_entry *entry = _fmt_entries[i];
+if (sample_fmt == entry->sample_fmt) {
+*fmt = AV_NE(entry->fmt_be, entry->fmt_le);
+return 0;
+}
+}
+
+fprintf(stderr,
+"sample format %s is not supported as output format\n",
+av_get_sample_fmt_name(sample_fmt));
+return -1;
+}
+
static void decode(AVCodecContext *dec_ctx, AVPacket *pkt, AVFrame *frame,
FILE *outfile)
{
@@ -172,6 +201,28 @@ int main(int argc, char **argv)
pkt->size = 0;
decode(c, pkt, decoded_frame, outfile);
+/* print output pcm infomations, because there have no metadata of pcm */
+enum AVSampleFormat sfmt = c->sample_fmt;
+int n_channels = c->channels;
+const char *fmt;
+
+if (av_sample_fmt_is_planar(sfmt)) {
+const char *packed = av_get_sample_fmt_name(sfmt);
+printf("Warning: the sample format the decoder produced is planar "
+ "(%s). This example will output the first channel only.\n",
+ packed ? packed : "?");
+sfmt = av_get_packed_sample_fmt(sfmt);
+}
+
+n_channels = c->channels;
+if ((ret = get_format_from_sample_fmt(, sfmt)) < 0)
+goto end;
+
+printf("Play the output audio file with the command:\n"
+ "ffplay -f %s -ac %d -ar %d %s\n",
+ fmt, n_channels, c->sample_rate,
+ outfilename);
+end:
fclose(outfile);
fclose(f);
--
2.17.2 (Apple Git-113)
___
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 v2] tools/target_dec_fuzzer: use refcounted packets
On 8/21/2019 6:15 AM, Tomas Härdin wrote:
> tis 2019-08-20 klockan 21:05 -0300 skrev James Almer:
>> Should reduce date copying considerably.
>>
>> Signed-off-by: James Almer
>> ---
>> Fixed a stupid mistake when checking the return value for av_new_packet().
>> Still untested.
>
> Works great for me. Should make fuzzing faster overall, better use of
> computing resources imo
>
>> @@ -186,6 +144,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
>> size) {
>> error("Failed memory allocation");
>>
>> ctx->max_pixels = maxpixels_per_frame; //To reduce false positive OOM
>> and hangs
>> +ctx->refcounted_frames = 1;
>
> Could maybe have a comment that this is also to reduce false positives,
> or that we want to focus on the new API rather than the old one
Sure.
>
>> @@ -240,7 +199,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
>> size) {
>> if (data + sizeof(fuzz_tag) > end)
>> data = end;
>>
>> -FDBPrepare(, , last, data - last);
>> +res = av_new_packet(, data - last);
>> +if (res < 0)
>> +error("Failed memory allocation");
>> +memcpy(parsepkt.data, last, data - last);
>
> Is there some way to avoid this copy?
I could create packets that point to a specific offset in the input
fuzzed buffer, but that will mean they will lack the padding required by
the API, not to mention the complete lack of control over said buffer's
lifetime. This could either generate no issues, or make things go really
wrong.
So to make proper use of the AVPacket API, i need to allocate their
reference counted buffers and populate them. After this patch the
behaviour of this fuzzer is essentially the same as if it was a
libavformat demuxer, creating packets out of an input file and
propagating them to libavcodec.
If the idea was to imitate an average library user's behavior, this
pretty much is it.
>
> /Tomas
>
> ___
> 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".
>
___
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] lavf: Add DICOM demuxer, lavc: Add DICOM decoder
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
> I have also uploaded DICOM samples here.
> https://1drv.ms/u/s!AosJ9_SrP4Tsh2WoPfQAI8cSsqUs?e=ZMd5fR
I found something else, with valgrind:
> +static int read_implicit_seq_item(AVFormatContext *s, DataElement *de)
> +{
> +int ret, f = -2, i = 0;
> +
> +de->bytes = av_malloc(MAX_UNDEF_LENGTH);
This return value needs to be checked!
> +for(; i < MAX_UNDEF_LENGTH; ++i) {
> +ret = avio_rl16(s->pb);
> +if (ret < 0)
> +return ret;
> +
> +if (ret == SEQ_GR_NB)
> +f = i;
> +if (ret == SEQ_ITEM_DEL_EL_NB && f == i - 1) {
> +avio_skip(s->pb, 4);
> +break;
> +}
> +bytestream_put_le16((uint8_t **)>bytes, ret);
> +}
> +de->VL = (i - 1) * 2;
> +return 0;
> +}
> +
> +static int read_seq(AVFormatContext *s, DataElement *de) {
> +int i = 0, ret;
> +DICOMContext *dicom = s->priv_data;
> +DataElement *seq_data = (DataElement *)av_malloc(sizeof(DataElement) *
> MAX_SEQ_LENGTH);
This is an array, right? There's a special av_malloc*() function for
allocating an array. (And the typecast is not required, IIUC.)
Furthermore, it is never freed.
Fixing free_de() as suggested alone is not sufficient either, the array
elements remain unfreed - those allocated here as seq_data[i]:
> +de->bytes = seq_data;
> +dicom->inseq = 1;
> +for (;i < MAX_SEQ_LENGTH; ++i) {
> +seq_data[i].bytes = NULL;
> +seq_data[i].desc = NULL;
> +seq_data[i].is_found = 0;
> +read_de_metainfo(s, seq_data + i);
> +if (seq_data[i].GroupNumber == SEQ_GR_NB
> +&& seq_data[i].ElementNumber == SEQ_DEL_EL_NB){
> +ret = i;
> +goto exit;
> +}
> +if (seq_data[i].VL == UNDEFINED_VL)
> +ret = read_implicit_seq_item(s, seq_data + i);
^ Here.
> +else
> +ret = read_de(s, seq_data + i);
> +if (ret < 0)
> +goto exit;
> +}
valgrind extract:
==20223== 10,000 bytes in 1 blocks are definitely lost in loss record 16 of 16
==20223==at 0x483AE4C: memalign (vg_replace_malloc.c:908)
==20223==by 0x483AF59: posix_memalign (vg_replace_malloc.c:1072)
==20223==by 0x5146F9: av_malloc (mem.c:87)
==20223==by 0x454C80: read_implicit_seq_item (dicomdec.c:371)
==20223==by 0x454C80: read_seq (dicomdec.c:410)
==20223==by 0x454C80: read_de_valuefield (dicomdec.c:424)
==20223==by 0x455312: dicom_read_packet (dicomdec.c:549)
==20223==by 0x4649C3: ff_read_packet (utils.c:856)
==20223==by 0x46594B: read_frame_internal (utils.c:1582)
==20223==by 0x46A001: avformat_find_stream_info (utils.c:3781)
==20223==by 0x4170BE: open_input_file (ffmpeg_opt.c:1126)
==20223==by 0x418A95: open_files (ffmpeg_opt.c:3275)
==20223==by 0x418A95: ffmpeg_parse_options (ffmpeg_opt.c:3315)
==20223==by 0x411875: main (ffmpeg.c:4872)
Furthermore, all your samples 000*.dcm give me the following error:
[dicom @ 0xfc26c0] Could not find codec parameters for stream 0 (Video: dicom,
none): unspecified size
Consider increasing the value for the 'analyzeduration' and 'probesize' options
Example:
[barsnick@paradise ffmpeg]$ ./ffmpeg_g -i 28.dcm -f null -
ffmpeg version N-94609-g406b3e902f Copyright (c) 2000-2019 the FFmpeg developers
built with gcc 8 (GCC)
configuration: --disable-doc --disable-everything --disable-network
--disable-vdpau --enable-indev=lavfi --enable-demuxer=dicom
--enable-muxer='null,hash,framehash,md5,framemd5'
--enable-encoder='wrapped_avframe,rawvideo,pcm_s16le'
--enable-decoder='rawvideo,pcm_f64le,dicom'
--enable-filter='color,testsrc,anoisesrc,null,aresample'
--enable-protocol='pipe,file'
libavutil 56. 33.100 / 56. 33.100
libavcodec 58. 56.100 / 58. 56.100
libavformat58. 32.101 / 58. 32.101
libavdevice58. 9.100 / 58. 9.100
libavfilter 7. 58.100 / 7. 58.100
libswscale 5. 6.100 / 5. 6.100
libswresample 3. 6.100 / 3. 6.100
[dicom @ 0xfc76c0] Could not find codec parameters for stream 0 (Video: dicom,
none): unspecified size
Consider increasing the value for the 'analyzeduration' and 'probesize' options
Input #0, dicom, from '28.dcm':
Metadata:
(0002,0001) File Meta Information Version: [Binary data]
(0002,0002) Media Storage SOP Class UID: 1.2.840.10008.5.1.4.1.1.2
(0002,0003) Media Storage SOP Inst UID:
1.3.6.1.4.1.14519.5.2.1.7014.4598.325035o92754234010375598120031
(0002,0010) Transfer Syntax UID: 1.2.840.10008.1.2.1
(0002,0012) Implementation Class UID: 1.2.40.0.13.1.1.1
(0002,0013) Implementation Version Name: dcm4che-1.4.35
Duration: N/A, start: 0.00, bitrate: N/A
Stream #0:0: Video: dicom, none, 1k tbr, 1k tbn
Output #0, null, to 'pipe:':
Output file #0 does not contain any stream
Having compiled the demuxer and decoder, I do now understand that DICOM
images can have multiple "frames".
Re: [FFmpeg-devel] [PATCH] lavf: Add DICOM demuxer, lavc: Add DICOM decoder
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
One more. Some more nitpicking around this. Please use valgrind with
all your samples and some demuxing and decoding command lines.
> +static int dicom_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +DICOMContext *dicom = s->priv_data;
> +int metadata = dicom->metadata, ret;
> +AVDictionary **st_md;
> +char *key, *value;
> +AVStream *st;
> +DataElement *de;
> +
> +if (s->nb_streams < 1) {
> +st = avformat_new_stream(s, NULL);
> +if (!st)
> +return AVERROR(ENOMEM);
> +st->codecpar->codec_id = AV_CODEC_ID_DICOM;
> +st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> +st->start_time = 0;
> +avpriv_set_pts_info(st, 64, 1, 1000);
> +if (dicom->window != -1)
> +st->codecpar->profile = dicom->window;
> +if (dicom->level != -1)
> +st->codecpar->level = dicom->level;
> +} else
> +st = s->streams[0];
> +
> +st_md = >metadata;
> +dicom->inheader = 0;
> +if (dicom->frame_nb > 1 && dicom->frame_nb <= dicom->nb_frames)
> +goto inside_pix_data;
> +
> +for (;;) {
> +ret = avio_feof(s->pb);
> +if (ret)
> +return AVERROR_EOF;
> +
> +de = alloc_de();
Allocation failure needs to be checked:
if (!de)
return AVERROR(ENOMEM);
> +ret = read_de_metainfo(s,de);
> +if (ret < 0)
> +goto exit;
> +
> +if (de->GroupNumber == PIXEL_GR_NB && de->ElementNumber ==
> PIXELDATA_EL_NB) {
> +dicom->frame_size = de->VL / dicom->nb_frames;
> +set_dec_extradata(s, st);
> +inside_pix_data:
> +if (av_new_packet(pkt, dicom->frame_size) < 0)
> +return AVERROR(ENOMEM);
This leaks de. Instead:
if (av_new_packet(pkt, dicom->frame_size) < 0) {
ret = AVERROR(ENOMEM);
goto exit;
}
> +pkt->pos = avio_tell(s->pb);
> +pkt->stream_index = 0;
> +pkt->size = dicom->frame_size;
> +pkt->duration = dicom->delay;
> +ret = avio_read(s->pb, pkt->data, dicom->frame_size);
> +if (ret < 0) {
> +av_packet_unref(pkt);
> +goto exit;
> +}
> +dicom->frame_nb ++;
> +return ret;
This leaks everything (i.e. de and pkt). Instead:
dicom->frame_nb ++;
av_packet_unref(pkt);
goto exit;
> +} else if (de->GroupNumber == IMAGE_GR_NB || de->GroupNumber ==
> MF_GR_NB) {
> +ret = read_de_valuefield(s, de);
> +if (ret < 0)
> +goto exit;
> +set_imagegroup_data(s, st, de);
> +set_multiframe_data(s, st, de);
> +} else {
> +if (metadata || de->VL == UNDEFINED_VL)
> +ret = read_de_valuefield(s, de);
> +else
> +ret = avio_skip(s->pb, de->VL); // skip other elements
> +if (ret < 0)
> +goto exit;
> +}
> +if (metadata) {
> +ret = dicom_dict_find_elem_info(de);
> +if (ret < 0)
> +goto end_de;
> +key = get_key_str(de);
> +value = get_val_str(de);
> +av_dict_set(st_md, key, value, AV_DICT_DONT_STRDUP_KEY |
> AV_DICT_DONT_STRDUP_VAL);
> +}
> +end_de:
> +free_de(de);
> +}
> +exit:
> +free_de(de);
> +if (ret < 0)
> +return ret;
> +return AVERROR_EOF;
> +}
And, as mentioned, "goto exit" can probably be a simple "break", but
that's up to you to verify that it still does the right thing.
Cheers,
Moritz
___
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".
