[FFmpeg-devel] [PATCH 1/1] avformat: mca: relax a condition check to be able to play certain files
From: liushuyu In certain mca files, the coefficient table is in the data section instead of the header section. In this case, the coefficient offset relative to the header ending marker is a negative value thus failing the original condition check at line 146. The new check just check if the coefficient offset is within the file range (since there is no way to know where the actual audio samples are without the correct header information). --- libavformat/mca.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavformat/mca.c b/libavformat/mca.c index 27cfb1c..5bb9a35 100644 --- a/libavformat/mca.c +++ b/libavformat/mca.c @@ -48,9 +48,9 @@ static int read_header(AVFormatContext *s) int64_t file_size = avio_size(s->pb); uint16_t version = 0; uint32_t header_size, data_size, data_offset, loop_start, loop_end, -nb_samples, nb_metadata, coef_offset = 0; +nb_samples, nb_metadata = 0; int ch, ret; -int64_t ret_size; +int64_t ret_size, coef_offset = 0; st = avformat_new_stream(s, NULL); if (!st) @@ -144,10 +144,10 @@ static int read_header(AVFormatContext *s) } // coefficient alignment = 0x30; metadata size = 0x14 -if (0x30 * par->channels + nb_metadata * 0x14 > header_size) -return AVERROR_INVALIDDATA; coef_offset = -header_size - 0x30 * par->channels + nb_metadata * 0x14; +(int64_t)header_size - 0x30 * par->channels + nb_metadata * 0x14; +if (coef_offset < 0 || coef_offset >= file_size) +return AVERROR_INVALIDDATA; st->start_time = 0; par->codec_id = AV_CODEC_ID_ADPCM_THP_LE; -- 2.28.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 0/1] avformat: mca: relax a condition check to be able to play
From: liushuyu In certain mca files, the coefficient table is in the data section instead of the header section. In this case, the coefficient offset relative to the header ending marker is a negative value thus failing the original condition check at line 146. The new check just check if the coefficient offset is within the file range (since there is no way to know where the actual audio samples are without the correct header information). I have also prepared some sample files for you to test: (Coefficient in data section) https://streams.videolan.org/ffmpeg/incoming/bgm_pot_metal_a1_a_1632.mca And at last, I found some old samples that uses older MCA container format: (MCA Version 3) https://streams.videolan.org/ffmpeg/incoming/s_bgm_4b_DownMix.mca liushuyu (1): avformat: mca: relax a condition check to be able to play certain files libavformat/mca.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.28.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 2/4] avcodec/cbs: allow cbs_read_fragment_content() to discard units
On 10/1/2020 8:57 PM, Mark Thompson wrote:
> On 20/09/2020 18:24, James Almer wrote:
>> The caller may not need all units in a fragment in reading only
>> scenarios. They
>> could in fact alter global state stored in the private CodedBitstreamType
>> fields in an undesirable way.
>> And unlike preventing decomposition of units, discarding can be done
>> based on
>> parsed values within the unit.
>>
>> Signed-off-by: James Almer
>> ---
>> libavcodec/cbs.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
>> index e73e18f398..363385b6f3 100644
>> --- a/libavcodec/cbs.c
>> +++ b/libavcodec/cbs.c
>> @@ -196,6 +196,11 @@ static int
>> cbs_read_fragment_content(CodedBitstreamContext *ctx,
>> av_log(ctx->log_ctx, AV_LOG_VERBOSE,
>> "Decomposition unimplemented for unit %d "
>> "(type %"PRIu32").\n", i, unit->type);
>> + } else if (err == AVERROR(EAGAIN)) {
>> + av_log(ctx->log_ctx, AV_LOG_VERBOSE,
>> + "Discarding unit %d "
>> + "(type %"PRIu32").\n", i, unit->type);
>> + ff_cbs_delete_unit(frag, i--);
>> } else if (err < 0) {
>> av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit
>> %d "
>> "(type %"PRIu32").\n", i, unit->type);
>>
>
> I don't really like how it is modifying the units in the fragment
> internally in the read function. EAGAIN as an "I didn't do anything
> with this" return from read_unit seems reasonable, but then deleting the
> unit here feels outside the intended meaning of the function.
>
> Would it make sense to push that further out somehow? E.g. av1dec.c
> ignoring undecomposed units, or maybe a separate function to do deletion
> under whatever conditions.
Seems overtly complicated for what ultimately will be the same result.
The fragment will have only the units that were not discarded before
being returned to the caller.
And the caller (av1dec) can't really know if a unit was decomposed or
not, assuming they are not removed from the fragment. In the case of
Patch 3/4, it will return EAGAIN after having filled the OBU header bits.
>
> - Mark
> ___
> 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 3/4] avcodec/cbs_av1: add an option to select an operating point
On 10/1/2020 9:23 PM, Mark Thompson wrote:
> On 20/09/2020 18:24, James Almer wrote:
>> This implements the function drop_obu() as defined in Setion 6.2.1
>> from the
>> spec.
>> In a reading only scenario, units that belong to an operating point the
>> caller doesn't want should not be parsed.
>>
>> Signed-off-by: James Almer
>> ---
>> libavcodec/cbs_av1.c | 18 +-
>> libavcodec/cbs_av1.h | 5 +
>> libavcodec/cbs_av1_syntax_template.c | 7 +++
>> 3 files changed, 29 insertions(+), 1 deletion(-)
>
> I think the AVClass and option of patch 1 and this seems like a sensible
> approach.
>
>> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
>> index dcf6c140ae..edacc29ca8 100644
>> --- a/libavcodec/cbs_av1.c
>> +++ b/libavcodec/cbs_av1.c
>> @@ -18,6 +18,7 @@
>> #include "libavutil/avassert.h"
>> #include "libavutil/pixfmt.h"
>> +#include "libavutil/opt.h"
>> #include "cbs.h"
>> #include "cbs_internal.h"
>> @@ -883,7 +884,7 @@ static int cbs_av1_read_unit(CodedBitstreamContext
>> *ctx,
>> int in_spatial_layer =
>> (priv->operating_point_idc >> (priv->spatial_id +
>> 8)) & 1;
>> if (!in_temporal_layer || !in_spatial_layer) {
>> - // Decoding will drop this OBU at this operating point.
>> + return AVERROR(EAGAIN); // drop_obu()
>> }
>> }
>> }
>> @@ -1238,10 +1239,25 @@ static const CodedBitstreamUnitTypeDescriptor
>> cbs_av1_unit_types[] = {
>> CBS_UNIT_TYPE_END_OF_LIST
>> };
>> +#define OFFSET(x) offsetof(CodedBitstreamAV1Context, x)
>> +static const AVOption cbs_av1_options[] = {
>> + { "oppoint", "Select an operating point of the scalable
>> bitstream", OFFSET(operating_point),
>> + AV_OPT_TYPE_INT, { .i64 = -1 }, -1,
>> AV1_MAX_OPERATING_POINTS - 1, 0 },
>
> "oppoint" isn't used as an abbreviation anywhere in the standard, only
> "op" (so, operating point point?). There isn't really any reason to
> shorten it, so just having "operating_point" would seem clearer.
It's used in the libdav1d decoder wrapper, so i wanted to keep it
consistent. Admittedly, CBS is not meant to face the user, so i guess i
can change it.
>
> For the help string, maybe something a little nicer like "Set operating
> point to select layers to decode from a scalable bitstream"?
Sure.
>
>> + { NULL }
>> +};
>> +
>> +static const AVClass cbs_av1_class = {
>> + .class_name = "cbs_av1",
>> + .item_name = av_default_item_name,
>> + .option = cbs_av1_options,
>> + .version = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> const CodedBitstreamType ff_cbs_type_av1 = {
>> .codec_id = AV_CODEC_ID_AV1,
>> .priv_data_size = sizeof(CodedBitstreamAV1Context),
>> + .priv_class = _av1_class,
>
> Not in the same order as patch 1. The way around doesn't matter, but
> keep it consistent.
Ok.
>
>> .unit_types = cbs_av1_unit_types,
>> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
>> index 7a0c08c596..27b44d68ff 100644
>> --- a/libavcodec/cbs_av1.h
>> +++ b/libavcodec/cbs_av1.h
>> @@ -416,6 +416,8 @@ typedef struct AV1ReferenceFrameState {
>> } AV1ReferenceFrameState;
>> typedef struct CodedBitstreamAV1Context {
>> + const AVClass *class;
>> +
>> AV1RawSequenceHeader *sequence_header;
>> AVBufferRef *sequence_header_ref;
>> @@ -443,6 +445,9 @@ typedef struct CodedBitstreamAV1Context {
>> int tile_rows;
>> AV1ReferenceFrameState ref[AV1_NUM_REF_FRAMES];
>> +
>> + // AVOptions
>> + int operating_point;
>> } CodedBitstreamAV1Context;
>> diff --git a/libavcodec/cbs_av1_syntax_template.c
>> b/libavcodec/cbs_av1_syntax_template.c
>> index bcaa334134..34d09fab68 100644
>> --- a/libavcodec/cbs_av1_syntax_template.c
>> +++ b/libavcodec/cbs_av1_syntax_template.c
>> @@ -186,6 +186,7 @@ static int
>> FUNC(decoder_model_info)(CodedBitstreamContext *ctx, RWContext *rw,
>> static int FUNC(sequence_header_obu)(CodedBitstreamContext *ctx,
>> RWContext *rw,
>> AV1RawSequenceHeader *current)
>> {
>> + CodedBitstreamAV1Context *priv = ctx->priv_data;
>> int i, err;
>> HEADER("Sequence Header");
>> @@ -253,6 +254,12 @@ static int
>> FUNC(sequence_header_obu)(CodedBitstreamContext *ctx, RWContext *rw,
>> }
>> }
>> }
>> + if (priv->operating_point >= 0) {
>> + int op_pt = 0;
>> + if (priv->operating_point <=
>> current->operating_points_cnt_minus_1)
>> + op_pt = priv->operating_point;
>> + priv->operating_point_idc = current->operating_point_idc[op_pt];
>> + }
>
> Would it maybe make more sense to put this near cbs_av1.c:900 to only
> apply to read rather than having it in the parsing template? (I know
> there is choose_operating_point() there in the standard, but it doesn't
> affect any
Re: [FFmpeg-devel] [PATCH 2/2] avformat/rtsp: allocate correct max number of pollfds
On Sun, 27. Sep 21:38, "zhilizhao(赵志立)" wrote:
>
>
> > On Sep 27, 2020, at 6:26 AM, Andriy Gelman wrote:
> >
> > From: Andriy Gelman
> >
> > There is one general rtsp connection plus two connections per stream
> > (rtp/rtcp).
> >
> > Signed-off-by: Andriy Gelman
> > ---
> > libavformat/rtsp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index 5d8491b74b..90f912feb9 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -1994,7 +1994,7 @@ static int udp_read_packet(AVFormatContext *s,
> > RTSPStream **prtsp_st,
> > int *fds = NULL, fdsnum, fdsidx;
> >
> > if (!p) {
> > -p = rt->p = av_malloc_array(2 * (rt->nb_rtsp_streams + 1),
> > sizeof(struct pollfd));
> > +p = rt->p = av_malloc_array(2 * rt->nb_rtsp_streams + 1,
> > sizeof(struct pollfd));
>
> LGTM. I’m not sure is it appropriate to modify sizeof(struct pollfd) to
> sizeof(*p) in this patch.
>
Zhao, thanks for reviewing. I'll apply the patch in a couple of days unless
there are more comments.
I was not going make sizeof(*p) change in this particular patch, but someone
correct me if you think otherwise.
Thanks,
--
Andriy
___
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 3/4] avcodec/cbs_av1: add an option to select an operating point
On 20/09/2020 18:24, James Almer wrote:
This implements the function drop_obu() as defined in Setion 6.2.1 from the
spec.
In a reading only scenario, units that belong to an operating point the
caller doesn't want should not be parsed.
Signed-off-by: James Almer
---
libavcodec/cbs_av1.c | 18 +-
libavcodec/cbs_av1.h | 5 +
libavcodec/cbs_av1_syntax_template.c | 7 +++
3 files changed, 29 insertions(+), 1 deletion(-)
I think the AVClass and option of patch 1 and this seems like a sensible
approach.
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index dcf6c140ae..edacc29ca8 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -18,6 +18,7 @@
#include "libavutil/avassert.h"
#include "libavutil/pixfmt.h"
+#include "libavutil/opt.h"
#include "cbs.h"
#include "cbs_internal.h"
@@ -883,7 +884,7 @@ static int cbs_av1_read_unit(CodedBitstreamContext *ctx,
int in_spatial_layer =
(priv->operating_point_idc >> (priv->spatial_id + 8)) & 1;
if (!in_temporal_layer || !in_spatial_layer) {
-// Decoding will drop this OBU at this operating point.
+return AVERROR(EAGAIN); // drop_obu()
}
}
}
@@ -1238,10 +1239,25 @@ static const CodedBitstreamUnitTypeDescriptor
cbs_av1_unit_types[] = {
CBS_UNIT_TYPE_END_OF_LIST
};
+#define OFFSET(x) offsetof(CodedBitstreamAV1Context, x)
+static const AVOption cbs_av1_options[] = {
+{ "oppoint", "Select an operating point of the scalable bitstream",
OFFSET(operating_point),
+ AV_OPT_TYPE_INT, { .i64 = -1 }, -1, AV1_MAX_OPERATING_POINTS
- 1, 0 },
"oppoint" isn't used as an abbreviation anywhere in the standard, only "op" (so,
operating point point?). There isn't really any reason to shorten it, so just having
"operating_point" would seem clearer.
For the help string, maybe something a little nicer like "Set operating point to
select layers to decode from a scalable bitstream"?
+{ NULL }
+};
+
+static const AVClass cbs_av1_class = {
+.class_name = "cbs_av1",
+.item_name = av_default_item_name,
+.option = cbs_av1_options,
+.version= LIBAVUTIL_VERSION_INT,
+};
+
const CodedBitstreamType ff_cbs_type_av1 = {
.codec_id = AV_CODEC_ID_AV1,
.priv_data_size= sizeof(CodedBitstreamAV1Context),
+.priv_class= _av1_class,
Not in the same order as patch 1. The way around doesn't matter, but keep it
consistent.
.unit_types= cbs_av1_unit_types,
diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 7a0c08c596..27b44d68ff 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -416,6 +416,8 @@ typedef struct AV1ReferenceFrameState {
} AV1ReferenceFrameState;
typedef struct CodedBitstreamAV1Context {
+const AVClass *class;
+
AV1RawSequenceHeader *sequence_header;
AVBufferRef *sequence_header_ref;
@@ -443,6 +445,9 @@ typedef struct CodedBitstreamAV1Context {
int tile_rows;
AV1ReferenceFrameState ref[AV1_NUM_REF_FRAMES];
+
+// AVOptions
+int operating_point;
} CodedBitstreamAV1Context;
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
index bcaa334134..34d09fab68 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -186,6 +186,7 @@ static int FUNC(decoder_model_info)(CodedBitstreamContext
*ctx, RWContext *rw,
static int FUNC(sequence_header_obu)(CodedBitstreamContext *ctx, RWContext
*rw,
AV1RawSequenceHeader *current)
{
+CodedBitstreamAV1Context *priv = ctx->priv_data;
int i, err;
HEADER("Sequence Header");
@@ -253,6 +254,12 @@ static int FUNC(sequence_header_obu)(CodedBitstreamContext
*ctx, RWContext *rw,
}
}
}
+if (priv->operating_point >= 0) {
+int op_pt = 0;
+if (priv->operating_point <= current->operating_points_cnt_minus_1)
+op_pt = priv->operating_point;
+priv->operating_point_idc = current->operating_point_idc[op_pt];
+}
Would it maybe make more sense to put this near cbs_av1.c:900 to only apply to
read rather than having it in the parsing template? (I know there is
choose_operating_point() there in the standard, but it doesn't affect any of
the sequence header.)
This probably wants an error message if the selected operating point isn't
valid, rather than silently ignoring the option.
fb(4, frame_width_bits_minus_1);
fb(4, frame_height_bits_minus_1);
Might be a good idea to document this in doc/decoders.texi.
- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
Re: [FFmpeg-devel] [PATCH 2/4] avcodec/cbs: allow cbs_read_fragment_content() to discard units
On 20/09/2020 18:24, James Almer wrote:
The caller may not need all units in a fragment in reading only scenarios. They
could in fact alter global state stored in the private CodedBitstreamType
fields in an undesirable way.
And unlike preventing decomposition of units, discarding can be done based on
parsed values within the unit.
Signed-off-by: James Almer
---
libavcodec/cbs.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index e73e18f398..363385b6f3 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -196,6 +196,11 @@ static int cbs_read_fragment_content(CodedBitstreamContext
*ctx,
av_log(ctx->log_ctx, AV_LOG_VERBOSE,
"Decomposition unimplemented for unit %d "
"(type %"PRIu32").\n", i, unit->type);
+} else if (err == AVERROR(EAGAIN)) {
+av_log(ctx->log_ctx, AV_LOG_VERBOSE,
+ "Discarding unit %d "
+ "(type %"PRIu32").\n", i, unit->type);
+ff_cbs_delete_unit(frag, i--);
} else if (err < 0) {
av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
"(type %"PRIu32").\n", i, unit->type);
I don't really like how it is modifying the units in the fragment internally in the read
function. EAGAIN as an "I didn't do anything with this" return from read_unit
seems reasonable, but then deleting the unit here feels outside the intended meaning of
the function.
Would it make sense to push that further out somehow? E.g. av1dec.c ignoring
undecomposed units, or maybe a separate function to do deletion under whatever
conditions.
- Mark
___
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 8/8] avcodec/av1dec: clean state on frame decoding errors
On 10/1/2020 7:15 PM, Mark Thompson wrote:
> On 29/09/2020 19:23, James Almer wrote:
>> On 9/29/2020 3:07 PM, Mark Thompson wrote:
>>> On 29/09/2020 17:17, James Almer wrote:
On 9/29/2020 12:57 PM, Mark Thompson wrote:
> On 25/09/2020 15:43, James Almer wrote:
>> Fixes: member access within null pointer of type 'TileGroupInfo' (aka
>> 'struct TileGroupInfo')
>> Fixes:
>> 25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616
>>
>>
>>
>>
>> Found-by: continuous fuzzing process
>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: James Almer
>> ---
>> libavcodec/av1dec.c | 30 --
>> 1 file changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>> index 07026b7aeb..e5cfc3f2f2 100644
>> --- a/libavcodec/av1dec.c
>> +++ b/libavcodec/av1dec.c
>> @@ -381,6 +381,20 @@ fail:
>> return AVERROR(ENOMEM);
>> }
>> +static void av1_decode_flush(AVCodecContext *avctx)
>> +{
>> + AV1DecContext *s = avctx->priv_data;
>> +
>> + for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++)
>> + av1_frame_unref(avctx, >ref[i]);
>> +
>> + av1_frame_unref(avctx, >cur_frame);
>> + s->raw_frame_header = NULL;
>> + s->raw_seq = NULL;
>> +
>> + ff_cbs_flush(s->cbc);
>> +}
>> +
>> static av_cold int av1_decode_free(AVCodecContext *avctx)
>> {
>> AV1DecContext *s = avctx->priv_data;
>> @@ -841,23 +855,11 @@ static int av1_decode_frame(AVCodecContext
>> *avctx, void *frame,
>> end:
>> ff_cbs_fragment_reset(>current_obu);
>> + if (ret < 0)
>> + av1_decode_flush(avctx);
>> return ret;
>> }
>> -static void av1_decode_flush(AVCodecContext *avctx)
>> -{
>> - AV1DecContext *s = avctx->priv_data;
>> -
>> - for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++)
>> - av1_frame_unref(avctx, >ref[i]);
>> -
>> - av1_frame_unref(avctx, >cur_frame);
>> - s->raw_frame_header = NULL;
>> - s->raw_seq = NULL;
>> -
>> - ff_cbs_flush(s->cbc);
>> -}
>> -
>> AVCodec ff_av1_decoder = {
>> .name = "av1",
>> .long_name = NULL_IF_CONFIG_SMALL("Alliance
>> for Open
>> Media AV1"),
>>
>
> This seems questionable - if you have some packet loss and lose an
> intermediate frame, I don't think you want to throw away all the old
> state since it may be able to continue from an earlier frame which was
> received correctly.
If a frame was not parsed correctly, then the reference frame state
from
then on will be invalid. Especially if ff_cbs_read_packet() is what
failed, considering everything after the corrupt OBU within the TU will
be discarded by CBS.
>>>
>>> Only the reference frame state which the frame would have modified has
>>> changed, which need not be all of it if the encoder knows it is going to
>>> be sending over an unreliable channel.
>>>
>>> Also intra refresh, though I haven't seen that implemented in an AV1
>>> encoder yet.
>>>
I can replace this to simply setting raw_frame_header to NULL if you
prefer, but for reference, dav1d (the decoder that hopefully will
eventually be ported into this native decoder) throws the entire state
away on a frame decoding failure.
>>>
>>> I think that answer is preferable, though I guess it could just be
>>> ignored for now and fixed when it comes up in future. (Presumably noone
>>> has tried to use dav1d for these sorts of cases yet.)
>>
>> Which option do you prefer, then? If i just set raw_frame_header to NULL
>> on failure and do nothing else, future frames will be parsed and
>> get_buffer() will still be called despite no pix_fmt being set (And thus
>> fail).
>
> I think you can avoid that by making sure that
> AV1ReferenceFrameState.valid is false for the frame that was lost?
get_buffer() is called even if no hwaccel is set because s->raw_seq will
not be NULL when parsing a frame after get_pixel_format() failed while
parsing the sequence header. This means get_current_frame() and then
av1_frame_alloc() will be reached. Hence setting it to NULL solving it.
Another option is to check for pix_fmt == NONE in av1_frame_alloc(),
right before the get_buffer() call, at least for the time being.
>
> Future frames will fail to parse if they refer directly to it, and if
> they don't then they will continue to work.
How would i do that in the cases where a ff_cbs_read_packet() failure
results in refresh_frame_flags not being read? I'll have no way to know
what slot/s in the ref frame array the frame in question was meant to
take. Not to mention all the other potential frames in the TU
[FFmpeg-devel] [WIP] Organisation process
Hello folks, I've documented what has been decided/discussed about the voting process and the committees. I'd like to know where should I commit this .md. In doc/? In a subfolder? somewhere else? I'd also like to have remarks about things that don't really match reality or are just wrong. (yes, the conduct document and the tech resolution documents are not here, but they will be shared here soon. and they might be trickier, yes) Best, # FFmpeg project ## Organisation The FFmpeg project is organized through a community working on global consensus. Decisions are taken by the ensemble of active members, through voting and are aided by two committees. ## General Assembly The ensemble of active members is called the General Assembly (GA). The General Assembly is sovereign and legitimate for all its decisions regarding the FFmpeg project. The General Assembly is made up of active contributors and the people who are added to this General Assembly through a vote. Contributors are considered "active contributors" if they have pushed more than 20 patches in the last 36 months in the main FFmpeg repository. Additional members are added to the General Assembly through a vote after being proposed by a member of the General Assembly. ## Voting Voting is done using a ranked voting system, currently running on https://vote.ffmpeg.org/ . Majority vote means more than 50% of the expressed ballots. ## Technical Committee The Technical Committee (TC) is here to arbitrage and take decisions when technical conflicts occur in the project. They will consider the merits of all the positions, judge them and take a decision. The TC is resolving technical conflicts but is not a technical steering committee. Decisions by the TC are binding for all the contributors. Decisions taken by the TC can be re-opened after 1 year or by a majority vote of the General Assembly, requested by one of the member of the GA. The TC is elected by the General Assembly for a duration of 1 year, and is composed of 5 members. Members can be reelected if they wish. A majority vote in the General Assembly can trigger a new election of the TC. The conflict resolution process is detailed in the [resolution process] document. ## Community committee The Community Committee (CC) is here to arbitrage and take decisions when inter-personal conflicts occur in the project. It will decide quickly and take actions, for the sake of the project. The CC can remove privileges of offending members, including removal of commit access and temporary ban from the community. Decisions taken by the CC can be re-opened after 1 year or by a majority vote of the General Assembly. Indefinite bans from the community must be confirmed by the General Assembly, in a majority vote. The CC is elected by the General Assembly for a duration of 1 year, and is composed of 5 members. Members can be reelected if they wish. A majority vote in the General Assembly can trigger a new election of the CC. The CC is governed by and responsible for the Code of Conduct. -- 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] [PATCH] avfilter/setparams: add FF_FILTER_FLAG_HWFRAME_AWARE
On 29/09/2020 19:49, Pavel Koshevoy wrote:
On Tue, Sep 29, 2020 at 12:14 PM Mark Thompson wrote:
On 29/09/2020 18:14, Pavel Koshevoy wrote:
On Tue, Sep 29, 2020 at 10:09 AM Mark Thompson wrote:
- Mark
It's pretty much this use case, except I'm not using ffmpeg cli but the
avfilter api to configure the filter chain, and I'm working with
AV_PIX_FMT_CUDA frames.
Perhaps I am mis-using the api, but my patch was sufficient for my needs:
```
bool
VideoFilterChain::setup_filter_links(int num_threads)
{
graph_ = avfilter_graph_alloc();
graph_->nb_threads = num_threads;
int err = avfilter_graph_parse2(graph_, filters_.c_str(), _,
_);
UL_FAIL_IF_AVERROR(err);
if (hw_frames_.ref_)
{
AVHWFramesContext * hw_frames_ctx =
hw_frames_.get();
AVBufferRef * device_ref = hw_frames_ctx->device_ref;
for (int i = 0; i < graph_->nb_filters; i++)
{
AVFilterContext * filter_ctx = graph_->filters[i];
UL_ASSERT(!filter_ctx->hw_device_ctx);
filter_ctx->hw_device_ctx = av_buffer_ref(device_ref);
bool found_hwdownload = strcmp(filter_ctx->filter->name,
"hwdownload") == 0;
if (found_hwdownload)
{
break;
}
for (int j = 0; j < filter_ctx->nb_outputs; j++)
{
AVFilterLink * link = filter_ctx->outputs[j];
UL_ASSERT(!link->hw_frames_ctx);
link->hw_frames_ctx =
av_buffer_ref(hw_frames_.ref_);
<
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavfilter/avfilter.h;h=99297ae798aa325ac37836a3a90d9a3f8e1e7a95;hb=HEAD#l497
Don't write to internal fields.
I'm not sure exactly what you're trying to do by writing the internal
context fields in this section, but I suspect that if you just remove it
entirely then the expected context propagation will happen and it will work.
Okay, if I fix my API mis-use and context propagation happens
automagically (idk where it would get the context in the 1st place,
yet)...
If you are putting hardware frames into a filter graph then it comes from
buffersrc. If you are using hwupload inside the filter graph then it gets made
there from the device you provide as AVFilterContext.hw_device_ctx.
won't that still leave me with setparams that is not
FF_FILTER_FLAG_HWFRAME_AWARE and avfilter_config_links would still fail?
Why would it fail?
- Mark
___
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 8/8] avcodec/av1dec: clean state on frame decoding errors
On 29/09/2020 19:23, James Almer wrote:
On 9/29/2020 3:07 PM, Mark Thompson wrote:
On 29/09/2020 17:17, James Almer wrote:
On 9/29/2020 12:57 PM, Mark Thompson wrote:
On 25/09/2020 15:43, James Almer wrote:
Fixes: member access within null pointer of type 'TileGroupInfo' (aka
'struct TileGroupInfo')
Fixes:
25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: James Almer
---
libavcodec/av1dec.c | 30 --
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 07026b7aeb..e5cfc3f2f2 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -381,6 +381,20 @@ fail:
return AVERROR(ENOMEM);
}
+static void av1_decode_flush(AVCodecContext *avctx)
+{
+ AV1DecContext *s = avctx->priv_data;
+
+ for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++)
+ av1_frame_unref(avctx, >ref[i]);
+
+ av1_frame_unref(avctx, >cur_frame);
+ s->raw_frame_header = NULL;
+ s->raw_seq = NULL;
+
+ ff_cbs_flush(s->cbc);
+}
+
static av_cold int av1_decode_free(AVCodecContext *avctx)
{
AV1DecContext *s = avctx->priv_data;
@@ -841,23 +855,11 @@ static int av1_decode_frame(AVCodecContext
*avctx, void *frame,
end:
ff_cbs_fragment_reset(>current_obu);
+ if (ret < 0)
+ av1_decode_flush(avctx);
return ret;
}
-static void av1_decode_flush(AVCodecContext *avctx)
-{
- AV1DecContext *s = avctx->priv_data;
-
- for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++)
- av1_frame_unref(avctx, >ref[i]);
-
- av1_frame_unref(avctx, >cur_frame);
- s->raw_frame_header = NULL;
- s->raw_seq = NULL;
-
- ff_cbs_flush(s->cbc);
-}
-
AVCodec ff_av1_decoder = {
.name = "av1",
.long_name = NULL_IF_CONFIG_SMALL("Alliance for Open
Media AV1"),
This seems questionable - if you have some packet loss and lose an
intermediate frame, I don't think you want to throw away all the old
state since it may be able to continue from an earlier frame which was
received correctly.
If a frame was not parsed correctly, then the reference frame state from
then on will be invalid. Especially if ff_cbs_read_packet() is what
failed, considering everything after the corrupt OBU within the TU will
be discarded by CBS.
Only the reference frame state which the frame would have modified has
changed, which need not be all of it if the encoder knows it is going to
be sending over an unreliable channel.
Also intra refresh, though I haven't seen that implemented in an AV1
encoder yet.
I can replace this to simply setting raw_frame_header to NULL if you
prefer, but for reference, dav1d (the decoder that hopefully will
eventually be ported into this native decoder) throws the entire state
away on a frame decoding failure.
I think that answer is preferable, though I guess it could just be
ignored for now and fixed when it comes up in future. (Presumably noone
has tried to use dav1d for these sorts of cases yet.)
Which option do you prefer, then? If i just set raw_frame_header to NULL
on failure and do nothing else, future frames will be parsed and
get_buffer() will still be called despite no pix_fmt being set (And thus
fail).
I think you can avoid that by making sure that AV1ReferenceFrameState.valid is
false for the frame that was lost?
Future frames will fail to parse if they refer directly to it, and if they
don't then they will continue to work.
If i also set raw_seq to NULL on failure then that can be avoided, but
it will be more or less functionally the same as this patch seeing that
no frame will be handled by this decoder until a new sequence header is
found, even if the CBS state was left intact.
Throwing away the sequence header doesn't seem right.
- Mark
___
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] rtsp: Fix infinite loop in listen mode with UDP transport
On Thu, 1 Oct 2020, Andriy Gelman wrote: On Thu, 01. Oct 22:00, Zhao Zhili wrote: On 10/1/20 4:15 AM, Martin Storsjö wrote: > On Wed, 30 Sep 2020, Zhao Zhili wrote: > > > Hi Martin, > > > > On 9/30/20 5:41 PM, Martin Storsjö wrote: > > > In listen mode with UDP transport, once the sender has sent > > > the TEARDOWN and closed the connection, poll will indicate that > > > one can read from the connection (indicating that the socket has > > > reached EOF and should be closed by the receiver as well). In this > > > case, parse_rtsp_message won't try to parse the command (because > > > it's no longer in state STREAMING), but previously just returned > > > zero. > > > > > > Prior to f6161fccf8c5720ceac1ed1df8ba60ff8fed69f5, this caused > > > udp_read_packet to return zero, which is treated as EOF by > > > read_packet. But after that commit, udp_read_packet would continue > > > if parse_rtsp_message didn't return an explicit error code. > > > > > > To keep the original behaviour from before that commit, more > > > explicitly return an error in parse_rtsp_message when in the wrong > > > state. > > > > > > Fixes: #8840 > > > --- > > > libavformat/rtsp.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > > index 5d8491b74b..ad12f2ae98 100644 > > > --- a/libavformat/rtsp.c > > > +++ b/libavformat/rtsp.c > > > @@ -1970,7 +1970,7 @@ static int parse_rtsp_message(AVFormatContext *s) > > > av_log(s, AV_LOG_WARNING, > > > "Unable to answer to TEARDOWN\n"); > > > } else > > > - return 0; > > > + return AVERROR_EOF; > > > > Does the else part needs the same fix? > > Which else part do you refer to? The else above (with the warning)? Yes > that bit looks a bit odd to me as well - your patch 2/2 looks like a > good fix for that. I mean the else below, especially /* XXX: parse message */ if (rt->state != RTSP_STATE_STREAMING) return 0; I did some tests with the rtsp server from: https://github.com/revmischa/rtsp-server This point can be reached with rt->state = RTSP_STATE_IDLE when the initial_pause option is set: ./ffmpeg -initial_pause 1 -i rtsp://127.0.0.1/abc -f null - Then it seems changing the return value in the above code would lead to unintended results. Thanks for testing this. Indeed, I'd rather treat that as a separate case. The listen mode is not very widely used, and this issue can be traced back to a regression, so that can be easily fixed in that context. For the other case you're pointing out, I don't have a concrete bug (the UDP mode seems to require waiting for a long timeout at the end though, but changing this return statement to return an error doesn't seem to help), and the normal non-listen mode code can be used in a huge variety of cases, many that aren't very easy to test (e.g. the code used to support RealRTSP, but I'm not sure if there's any publicly available test clips/servers for that any longer - multimediawiki used to list some, but I tested them last time close to a decade ago, and then there might have been zero or one of them still responding). So for that code, I'd tread much more carefully... // Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] rtsp: Fix infinite loop in listen mode with UDP transport
On Thu, 01. Oct 22:00, Zhao Zhili wrote: > > On 10/1/20 4:15 AM, Martin Storsjö wrote: > > On Wed, 30 Sep 2020, Zhao Zhili wrote: > > > > > Hi Martin, > > > > > > On 9/30/20 5:41 PM, Martin Storsjö wrote: > > > > In listen mode with UDP transport, once the sender has sent > > > > the TEARDOWN and closed the connection, poll will indicate that > > > > one can read from the connection (indicating that the socket has > > > > reached EOF and should be closed by the receiver as well). In this > > > > case, parse_rtsp_message won't try to parse the command (because > > > > it's no longer in state STREAMING), but previously just returned > > > > zero. > > > > > > > > Prior to f6161fccf8c5720ceac1ed1df8ba60ff8fed69f5, this caused > > > > udp_read_packet to return zero, which is treated as EOF by > > > > read_packet. But after that commit, udp_read_packet would continue > > > > if parse_rtsp_message didn't return an explicit error code. > > > > > > > > To keep the original behaviour from before that commit, more > > > > explicitly return an error in parse_rtsp_message when in the wrong > > > > state. > > > > > > > > Fixes: #8840 > > > > --- > > > > libavformat/rtsp.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > > > index 5d8491b74b..ad12f2ae98 100644 > > > > --- a/libavformat/rtsp.c > > > > +++ b/libavformat/rtsp.c > > > > @@ -1970,7 +1970,7 @@ static int parse_rtsp_message(AVFormatContext *s) > > > > av_log(s, AV_LOG_WARNING, > > > > "Unable to answer to TEARDOWN\n"); > > > > } else > > > > - return 0; > > > > + return AVERROR_EOF; > > > > > > Does the else part needs the same fix? > > > > Which else part do you refer to? The else above (with the warning)? Yes > > that bit looks a bit odd to me as well - your patch 2/2 looks like a > > good fix for that. > > I mean the else below, especially > > /* XXX: parse message */ > if (rt->state != RTSP_STATE_STREAMING) > return 0; I did some tests with the rtsp server from: https://github.com/revmischa/rtsp-server This point can be reached with rt->state = RTSP_STATE_IDLE when the initial_pause option is set: ./ffmpeg -initial_pause 1 -i rtsp://127.0.0.1/abc -f null - Then it seems changing the return value in the above code would lead to unintended results. -- Andriy ___ 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 3/3] avformat/mxfdec: Read Apple private Content Light Level from MXF
On Thu, Oct 01, 2020 at 03:29:19PM +0100, Harry Mallon wrote: > > > > > On 30 Sep 2020, at 08:32, Michael Niedermayer > > wrote: > > > > On Thu, Sep 17, 2020 at 10:49:31PM +0200, Tomas Härdin wrote: > >> mån 2020-09-14 klockan 12:23 +0200 skrev Tomas Härdin: > >>> ons 2020-09-09 klockan 15:56 +0100 skrev Harry Mallon: > * As embedded by Apple Compressor > > Signed-off-by: Harry Mallon > --- > libavformat/mxfdec.c| 27 + > tests/fate/mxf.mak | 4 + > tests/ref/fate/mxf-probe-applehdr10 | 169 > > >>> > >>> Sweet, I don't have to write the test myself .) > >>> > >>> Just ran FATE, the entire patch set works fine. We just need to get > >>> that sample into the sample suite then all three of them can be > >>> pushed. > >>> I'll see what I can do. > >> > >> FATE suite updated, FATE passes -> patchset pushed > > > > fails on big endian > > > > --- src/tests/ref/fate/mxf-probe-applehdr10 2020-09-28 23:21:12.291897976 > > +0200 > > +++ tests/data/fate/mxf-probe-applehdr102020-09-30 09:31:38.614653806 > > +0200 > > @@ -14,7 +14,7 @@ > > has_b_frames=0 > > sample_aspect_ratio=1:1 > > display_aspect_ratio=16:9 > > -pix_fmt=yuv422p10le > > +pix_fmt=yuv422p10be > > level=-99 > > color_range=tv > > color_space=bt2020nc > > Test mxf-probe-applehdr10 failed. Look at > > tests/data/fate/mxf-probe-applehdr10.err for details. > > src/tests/Makefile:255: recipe for target 'fate-mxf-probe-applehdr10' failed > > make: *** [fate-mxf-probe-applehdr10] Error 1 > > It seems fair that the pixel type is in native endian. maybe but the endianness of the decoder output doesnt belong in the comparission > I'm not really familiar enough with FATE to provide a patch to fix this > though. Do any other FATE tests have wildcards or two versions for big and > little endian? i dont see another probe reference file that contains a le/be format we had le/be issues in other places though where they where fixed by forcing a format with specific endianness in the test IIRC thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire 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] fate: add scale filters for big-endian architectures.
On Wed, Sep 30, 2020 at 08:44:24PM +0200, Nicolas George wrote: > Michael Niedermayer (12020-09-30): > > > confirmed to work on qemu mips > > > so should be ok > > Sorry, I missed that part of your mail. Pushed. > > > ping, big endian is still broken > > A few minutes ago, I still did not see these failures on > fate.ffmpeg.org. Any idea why? the machiene the mips fate client VM was on crashed and i forgot to restart all parts > > > also i realize that this has a disadvantage, and that is that it adds > > quite a bit of extra human work. Because now every time a fate test > > is added theres an additional thing to consider which is specific > > to big endian and which most developers cannot easily test > > I think you are painting this too darkly, the pattern is quite simple: > tests that output high bit depth require a final conversion filter. We > can be careful when reviewing commits that add tests. And if it does not > work, there is something weirdly broken that needs to be investigated, > so it is a good thing we know about it. its better to automate it. patchwork already tests submitted patches, this should be extended to cover big endian as few people are able to easily test that thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB During times of universal deceit, telling the truth becomes a revolutionary act. -- George Orwell 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] avcodec: add Cintel RAW decoder
Paul B Mahol:
> Signed-off-by: Paul B Mahol
> ---
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/codec_desc.c | 7 +
> libavcodec/codec_id.h | 1 +
> libavcodec/cri.c| 300
> libavformat/img2.c | 1 +
> 6 files changed, 311 insertions(+)
> create mode 100644 libavcodec/cri.c
>
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 816d87ba60..488cb158ef 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -269,6 +269,7 @@ OBJS-$(CONFIG_COMFORTNOISE_DECODER)+= cngdec.o
> celp_filters.o
> OBJS-$(CONFIG_COMFORTNOISE_ENCODER)+= cngenc.o
> OBJS-$(CONFIG_COOK_DECODER)+= cook.o
> OBJS-$(CONFIG_CPIA_DECODER)+= cpia.o
> +OBJS-$(CONFIG_CRI_DECODER) += cri.o
> OBJS-$(CONFIG_CSCD_DECODER)+= cscd.o
> OBJS-$(CONFIG_CYUV_DECODER)+= cyuv.o
> OBJS-$(CONFIG_DCA_DECODER) += dcadec.o dca.o dcadata.o dcahuff.o
> \
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 2b580b66cf..52fc415815 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -82,6 +82,7 @@ extern AVCodec ff_cllc_decoder;
> extern AVCodec ff_comfortnoise_encoder;
> extern AVCodec ff_comfortnoise_decoder;
> extern AVCodec ff_cpia_decoder;
> +extern AVCodec ff_cri_decoder;
> extern AVCodec ff_cscd_decoder;
> extern AVCodec ff_cyuv_decoder;
> extern AVCodec ff_dds_decoder;
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 1246dc2b96..01c0eca7b4 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -1812,6 +1812,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
> .long_name = NULL_IF_CONFIG_SMALL("Argonaut Games Video"),
> .props = AV_CODEC_PROP_LOSSY,
> },
> +{
> +.id= AV_CODEC_ID_CRI,
> +.type = AVMEDIA_TYPE_VIDEO,
> +.name = "cri",
> +.long_name = NULL_IF_CONFIG_SMALL("Cintel RAW"),
> +.props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY |
> AV_CODEC_PROP_LOSSLESS,
> +},
>
> /* various PCM "codecs" */
> {
> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
> index 21444f9ce3..e933f7664a 100644
> --- a/libavcodec/codec_id.h
> +++ b/libavcodec/codec_id.h
> @@ -300,6 +300,7 @@ enum AVCodecID {
> AV_CODEC_ID_PHOTOCD,
> AV_CODEC_ID_IPU,
> AV_CODEC_ID_ARGO,
> +AV_CODEC_ID_CRI,
>
> /* various PCM "codecs" */
> AV_CODEC_ID_FIRST_AUDIO = 0x1, ///< A dummy id pointing at the
> start of audio codecs
> diff --git a/libavcodec/cri.c b/libavcodec/cri.c
> new file mode 100644
> index 00..4bd0262255
> --- /dev/null
> +++ b/libavcodec/cri.c
> @@ -0,0 +1,300 @@
> +/*
> + * CRI image decoder
> + *
> + * Copyright (c) 2020 Paul B Mahol
> + *
> + * 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
> + */
> +
> +/**
> + * @file
> + * Cintel RAW image decoder
> + */
> +
> +#define BITSTREAM_READER_LE
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/opt.h"
These three headers seem to be unnecessary.
> +#include "avcodec.h"
> +#include "bytestream.h"
> +#include "get_bits.h"
> +#include "internal.h"
> +#include "thread.h"
> +
> +typedef struct CRIContext {
> +AVCodecContext *jpeg_avctx; // wrapper context for MJPEG
> +AVFrame *jpgframe;// decoded JPEG tile
> +
> +GetByteContext gb;
> +int color_model;
> +const uint8_t *data;
> +unsigned data_size;
> +uint64_t tile_size[4];
> +} CRIContext;
> +
> +static av_cold int cri_decode_init(AVCodecContext *avctx)
> +{
> +CRIContext *s = avctx->priv_data;
> +const AVCodec *codec;
> +int ret;
> +
> +s->jpgframe = av_frame_alloc();
This leaks if an error happens later.
> +if (!s->jpgframe)
> +return AVERROR(ENOMEM);
> +
> +codec = avcodec_find_decoder(AV_CODEC_ID_MJPEG);
> +if (!codec)
> +return AVERROR_BUG;
> +s->jpeg_avctx = avcodec_alloc_context3(codec);
> +if (!s->jpeg_avctx)
> +return AVERROR(ENOMEM);
> +s->jpeg_avctx->flags = avctx->flags;
> +s->jpeg_avctx->flags2 =
[FFmpeg-devel] [PATCH] avcodec: add Cintel RAW decoder
Signed-off-by: Paul B Mahol
---
libavcodec/Makefile | 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/codec_desc.c | 7 +
libavcodec/codec_id.h | 1 +
libavcodec/cri.c| 300
libavformat/img2.c | 1 +
6 files changed, 311 insertions(+)
create mode 100644 libavcodec/cri.c
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 816d87ba60..488cb158ef 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -269,6 +269,7 @@ OBJS-$(CONFIG_COMFORTNOISE_DECODER)+= cngdec.o
celp_filters.o
OBJS-$(CONFIG_COMFORTNOISE_ENCODER)+= cngenc.o
OBJS-$(CONFIG_COOK_DECODER)+= cook.o
OBJS-$(CONFIG_CPIA_DECODER)+= cpia.o
+OBJS-$(CONFIG_CRI_DECODER) += cri.o
OBJS-$(CONFIG_CSCD_DECODER)+= cscd.o
OBJS-$(CONFIG_CYUV_DECODER)+= cyuv.o
OBJS-$(CONFIG_DCA_DECODER) += dcadec.o dca.o dcadata.o dcahuff.o \
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 2b580b66cf..52fc415815 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -82,6 +82,7 @@ extern AVCodec ff_cllc_decoder;
extern AVCodec ff_comfortnoise_encoder;
extern AVCodec ff_comfortnoise_decoder;
extern AVCodec ff_cpia_decoder;
+extern AVCodec ff_cri_decoder;
extern AVCodec ff_cscd_decoder;
extern AVCodec ff_cyuv_decoder;
extern AVCodec ff_dds_decoder;
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 1246dc2b96..01c0eca7b4 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1812,6 +1812,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
.long_name = NULL_IF_CONFIG_SMALL("Argonaut Games Video"),
.props = AV_CODEC_PROP_LOSSY,
},
+{
+.id= AV_CODEC_ID_CRI,
+.type = AVMEDIA_TYPE_VIDEO,
+.name = "cri",
+.long_name = NULL_IF_CONFIG_SMALL("Cintel RAW"),
+.props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY |
AV_CODEC_PROP_LOSSLESS,
+},
/* various PCM "codecs" */
{
diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
index 21444f9ce3..e933f7664a 100644
--- a/libavcodec/codec_id.h
+++ b/libavcodec/codec_id.h
@@ -300,6 +300,7 @@ enum AVCodecID {
AV_CODEC_ID_PHOTOCD,
AV_CODEC_ID_IPU,
AV_CODEC_ID_ARGO,
+AV_CODEC_ID_CRI,
/* various PCM "codecs" */
AV_CODEC_ID_FIRST_AUDIO = 0x1, ///< A dummy id pointing at the
start of audio codecs
diff --git a/libavcodec/cri.c b/libavcodec/cri.c
new file mode 100644
index 00..4bd0262255
--- /dev/null
+++ b/libavcodec/cri.c
@@ -0,0 +1,300 @@
+/*
+ * CRI image decoder
+ *
+ * Copyright (c) 2020 Paul B Mahol
+ *
+ * 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
+ */
+
+/**
+ * @file
+ * Cintel RAW image decoder
+ */
+
+#define BITSTREAM_READER_LE
+
+#include "libavutil/avassert.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/opt.h"
+#include "avcodec.h"
+#include "bytestream.h"
+#include "get_bits.h"
+#include "internal.h"
+#include "thread.h"
+
+typedef struct CRIContext {
+AVCodecContext *jpeg_avctx; // wrapper context for MJPEG
+AVFrame *jpgframe;// decoded JPEG tile
+
+GetByteContext gb;
+int color_model;
+const uint8_t *data;
+unsigned data_size;
+uint64_t tile_size[4];
+} CRIContext;
+
+static av_cold int cri_decode_init(AVCodecContext *avctx)
+{
+CRIContext *s = avctx->priv_data;
+const AVCodec *codec;
+int ret;
+
+s->jpgframe = av_frame_alloc();
+if (!s->jpgframe)
+return AVERROR(ENOMEM);
+
+codec = avcodec_find_decoder(AV_CODEC_ID_MJPEG);
+if (!codec)
+return AVERROR_BUG;
+s->jpeg_avctx = avcodec_alloc_context3(codec);
+if (!s->jpeg_avctx)
+return AVERROR(ENOMEM);
+s->jpeg_avctx->flags = avctx->flags;
+s->jpeg_avctx->flags2 = avctx->flags2;
+s->jpeg_avctx->dct_algo = avctx->dct_algo;
+s->jpeg_avctx->idct_algo = avctx->idct_algo;
+ret = ff_codec_open2_recursive(s->jpeg_avctx, codec, NULL);
+if (ret < 0)
+return ret;
+
+return 0;
+}
+
+static int cri_decode_frame(AVCodecContext *avctx, void *data,
+int *got_frame, AVPacket *avpkt)
+{
+CRIContext *s =
Re: [FFmpeg-devel] [PATCH v2 3/3] avformat/mxfdec: Read Apple private Content Light Level from MXF
> On 30 Sep 2020, at 08:32, Michael Niedermayer wrote: > > On Thu, Sep 17, 2020 at 10:49:31PM +0200, Tomas Härdin wrote: >> mån 2020-09-14 klockan 12:23 +0200 skrev Tomas Härdin: >>> ons 2020-09-09 klockan 15:56 +0100 skrev Harry Mallon: * As embedded by Apple Compressor Signed-off-by: Harry Mallon --- libavformat/mxfdec.c| 27 + tests/fate/mxf.mak | 4 + tests/ref/fate/mxf-probe-applehdr10 | 169 >>> >>> Sweet, I don't have to write the test myself .) >>> >>> Just ran FATE, the entire patch set works fine. We just need to get >>> that sample into the sample suite then all three of them can be >>> pushed. >>> I'll see what I can do. >> >> FATE suite updated, FATE passes -> patchset pushed > > fails on big endian > > --- src/tests/ref/fate/mxf-probe-applehdr10 2020-09-28 23:21:12.291897976 > +0200 > +++ tests/data/fate/mxf-probe-applehdr10 2020-09-30 09:31:38.614653806 > +0200 > @@ -14,7 +14,7 @@ > has_b_frames=0 > sample_aspect_ratio=1:1 > display_aspect_ratio=16:9 > -pix_fmt=yuv422p10le > +pix_fmt=yuv422p10be > level=-99 > color_range=tv > color_space=bt2020nc > Test mxf-probe-applehdr10 failed. Look at > tests/data/fate/mxf-probe-applehdr10.err for details. > src/tests/Makefile:255: recipe for target 'fate-mxf-probe-applehdr10' failed > make: *** [fate-mxf-probe-applehdr10] Error 1 It seems fair that the pixel type is in native endian. I'm not really familiar enough with FATE to provide a patch to fix this though. Do any other FATE tests have wildcards or two versions for big and little endian? > > [...] > Harry ___ 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] rtsp: Fix infinite loop in listen mode with UDP transport
On 10/1/20 4:15 AM, Martin Storsjö wrote: On Wed, 30 Sep 2020, Zhao Zhili wrote: Hi Martin, On 9/30/20 5:41 PM, Martin Storsjö wrote: In listen mode with UDP transport, once the sender has sent the TEARDOWN and closed the connection, poll will indicate that one can read from the connection (indicating that the socket has reached EOF and should be closed by the receiver as well). In this case, parse_rtsp_message won't try to parse the command (because it's no longer in state STREAMING), but previously just returned zero. Prior to f6161fccf8c5720ceac1ed1df8ba60ff8fed69f5, this caused udp_read_packet to return zero, which is treated as EOF by read_packet. But after that commit, udp_read_packet would continue if parse_rtsp_message didn't return an explicit error code. To keep the original behaviour from before that commit, more explicitly return an error in parse_rtsp_message when in the wrong state. Fixes: #8840 --- libavformat/rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 5d8491b74b..ad12f2ae98 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1970,7 +1970,7 @@ static int parse_rtsp_message(AVFormatContext *s) av_log(s, AV_LOG_WARNING, "Unable to answer to TEARDOWN\n"); } else - return 0; + return AVERROR_EOF; Does the else part needs the same fix? Which else part do you refer to? The else above (with the warning)? Yes that bit looks a bit odd to me as well - your patch 2/2 looks like a good fix for that. I mean the else below, especially /* XXX: parse message */ if (rt->state != RTSP_STATE_STREAMING) return 0; Otherwise, I'm OK with the patch. I tried a similar approach in http://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/267437.html. I'm less keen on that approach. As the problem is with listen mode, I'd cautiously avoid changing code that is general to both modes. The intended behavior (e.g., return value) of parse_rtsp_message is not quite clear to me, could you help improve it, like adding some documentation? Well I'm not sure how much there is to document. It's a static function that is called from one single place in the code, so the documentation of it is the code surrounding the single call to it. Ideally it'd follow common conventions like returning a negative value on error and zero/positive on success that one should continue from. // Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ 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 v3] avformat/libsrt: print streamid at client
Print the SRT streamid at the client.
Logged to the context and changed into to verbose.
Signed-off-by: raghavendra
---
libavformat/libsrt.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
index 4025b24976..2cf5f57304 100644
--- a/libavformat/libsrt.c
+++ b/libavformat/libsrt.c
@@ -359,6 +359,13 @@ static int libsrt_set_options_pre(URLContext *h, int fd)
return 0;
}
+static void libsrt_dump_streamid(URLContext *h, int fd)
+{
+char streamid[512];
+int optlen = sizeof(streamid);
+if (!libsrt_getsockopt(h, fd, SRTO_STREAMID, "SRTO_STREAMID", streamid,
))
+av_log(h, AV_LOG_VERBOSE, "srt_streamid : %s\n", streamid);
+}
static int libsrt_setup(URLContext *h, const char *uri, int flags)
{
@@ -442,6 +449,8 @@ static int libsrt_setup(URLContext *h, const char *uri, int
flags)
goto fail1;
listen_fd = fd;
fd = ret;
+// dump srt streamid at client
+libsrt_dump_streamid(h, fd);
} else {
if (s->mode == SRT_MODE_RENDEZVOUS) {
ret = srt_bind(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
--
2.25.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 v2] avformat/movenc: Don't free AVCodecParameters manually
Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> Sorry, I missed that my tree is dirty before sending the last patch.
>
> libavformat/movenc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 20768cd45f..63adae5e0a 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -6255,9 +6255,7 @@ static void mov_free(AVFormatContext *s)
> int i;
>
> if (mov->chapter_track) {
> -if (mov->tracks[mov->chapter_track].par)
> -av_freep(>tracks[mov->chapter_track].par->extradata);
> -av_freep(>tracks[mov->chapter_track].par);
> +avcodec_parameters_free(>tracks[mov->chapter_track].par);
> }
>
> for (i = 0; i < mov->nb_streams; i++) {
>
Will apply tomorrow unless there are objections.
- 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 1/3] avformat/movenc: Free old vos_data before overwriting it
Andreas Rheinhardt: > Otherwise the old data leaks whenever extradata needs to be rewritten > (e.g. when encoding FLAC with our encoder that sends an updated > extradata packet at the end). > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/movenc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index a90bbfa458..c53be74a64 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -6978,6 +6978,7 @@ static int mov_write_trailer(AVFormatContext *s) > AVCodecParameters *par = track->par; > > track->vos_len = par->extradata_size; > +av_freep(>vos_data); > track->vos_data = av_malloc(track->vos_len + > AV_INPUT_BUFFER_PADDING_SIZE); > if (!track->vos_data) > return AVERROR(ENOMEM); > Will apply this patchset tomorrow unless there are objections. - 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] avformat/movenc: Fix stack overflow when remuxing timecode tracks
Martin Storsjö:
> On Wed, 30 Sep 2020, Andreas Rheinhardt wrote:
>
>> There are two possible kinds of timecode tracks (with tag "tmcd") in the
>> mov muxer: Tracks created internally by the muxer and timecode tracks
>> sent by the user. If any of the latter exists, the former are
>> deactivated. The former all belong to another track, the source
>> track; the latter don't have a source track set, but the index of the
>> source track is initially zeroed by av_mallocz_array().
>
> Would it be worthwhile to explicitly initialize these to e.g. -1, to
> make src_track clearer?
>
I wouldn't object to this, but this could only be done after fixing the
second point below. (Also note that there is actually an overflow
problem with nb_streams when AVFormatContext.nb_streams is huge (it's
technically an unsigned, but restricted to the range of int) and if you
use -1, you can't really solve the overflow problem by using an unsigned.)
>> This is a problem since 3d894db700cc1e360a7a75ab9ac8bf67ac6670a3: Said
>> commit added a function that calculates the duration of tracks and the
>> duration of timecode tracks is calculated by rescaling the duration
>> (calculated by the very same function) of the source track. This gives
>> an infinite recursion if the first track (the one that will be treated
>> as source track for all timecode tracks) is a timecode track itself,
>> leading to a stack overflow.
>>
>> This commit fixes this by not using the nonexistent source track
>> when calculating the duration of timecode tracks not created internally
>> by the mov muxer.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> 1. Remuxing timecode tracks is currently impossible for mp4 (the codec
>> tag validation fails); I wonder whether this is actually intended given
>> that there is a warning that timecode metadata is ignored if an explicit
>> timecode track is present.
>> 2. There is another place where the src_track field is used even when a
>> timecode track doesn't have a src_track: When writing the moov tag
>> (lines 4156-4166). This will probably also need to be fixed, but it is
>> not dangerous.
>> 3. There is btw no validation that a track with "tmcd" tag is a data
>> stream.
>>
>> libavformat/movenc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 2006fcee4b..265465f97b 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -2901,7 +2901,7 @@ static int mov_write_minf_tag(AVFormatContext
>> *s, AVIOContext *pb, MOVMuxContext
>>
>> static int64_t calc_pts_duration(MOVMuxContext *mov, MOVTrack *track)
>> {
>> - if (track->tag == MKTAG('t','m','c','d')) {
>> + if (track->tag == MKTAG('t','m','c','d') && mov->nb_meta_tmcd) {
>> // tmcd tracks gets track_duration set in mov_write_moov_tag from
>> // another track's duration, while the end_pts may be left at
>> zero.
>
> This fix in itself is probably good and safe, so LGTM.
>
Thanks, applied.
- 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] avcodec/dolby_e: set constant frame_size
Fixes pts generation. --- libavcodec/dolby_e.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/dolby_e.c b/libavcodec/dolby_e.c index 429612ec08..b0e6d6aee3 100644 --- a/libavcodec/dolby_e.c +++ b/libavcodec/dolby_e.c @@ -577,6 +577,7 @@ static int filter_frame(DBEContext *s, AVFrame *frame) reorder = ch_reorder_n; frame->nb_samples = FRAME_SAMPLES; +s->avctx->frame_size = FRAME_SAMPLES; if ((ret = ff_get_buffer(s->avctx, frame, 0)) < 0) return ret; -- 2.27.0.windows.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 v2] avformat/libsrt: print streamid at client
On Thu, Oct 01, 2020 at 14:13:14 +0530, raghavendra wrote:
> Print the SRT streamid at the client.
> Logged to the context and changed info to verbose.
No, this is a patch on top of your original patch. That is not
acceptable. You need to change the original commit and resubmit that.
> if(!libsrt_getsockopt(h, fd, SRTO_STREAMID, "SRTO_STREAMID", streamid,
> ))
^ please fix style: "if ("
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".
Re: [FFmpeg-devel] [PATCH] avcodec/qsv: Fix leak of options on error
Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/qsv.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
> index 7816d2f93c..6e3154e1a3 100644
> --- a/libavcodec/qsv.c
> +++ b/libavcodec/qsv.c
> @@ -361,6 +361,7 @@ static int ff_qsv_set_display_handle(AVCodecContext
> *avctx, QSVSession *qs)
> av_dict_set(_device_opts, "driver","iHD", 0);
>
> ret = av_hwdevice_ctx_create(>va_device_ref, AV_HWDEVICE_TYPE_VAAPI,
> NULL, child_device_opts, 0);
> +av_dict_free(_device_opts);
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR, "Failed to create a VAAPI device.\n");
> return ret;
> @@ -375,8 +376,6 @@ static int ff_qsv_set_display_handle(AVCodecContext
> *avctx, QSVSession *qs)
> }
> }
>
> -av_dict_free(_device_opts);
> -
> return 0;
> }
> #endif //AVCODEC_QSV_LINUX_SESSION_HANDLE
>
Will apply this tomorrow unless there are objections.
- 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] avformat/movenc: Fix stack overflow when remuxing timecode tracks
On Wed, 30 Sep 2020, Andreas Rheinhardt wrote:
There are two possible kinds of timecode tracks (with tag "tmcd") in the
mov muxer: Tracks created internally by the muxer and timecode tracks
sent by the user. If any of the latter exists, the former are
deactivated. The former all belong to another track, the source
track; the latter don't have a source track set, but the index of the
source track is initially zeroed by av_mallocz_array().
Would it be worthwhile to explicitly initialize these to e.g. -1, to make
src_track clearer?
This is a problem since 3d894db700cc1e360a7a75ab9ac8bf67ac6670a3: Said
commit added a function that calculates the duration of tracks and the
duration of timecode tracks is calculated by rescaling the duration
(calculated by the very same function) of the source track. This gives
an infinite recursion if the first track (the one that will be treated
as source track for all timecode tracks) is a timecode track itself,
leading to a stack overflow.
This commit fixes this by not using the nonexistent source track
when calculating the duration of timecode tracks not created internally
by the mov muxer.
Signed-off-by: Andreas Rheinhardt
---
1. Remuxing timecode tracks is currently impossible for mp4 (the codec
tag validation fails); I wonder whether this is actually intended given
that there is a warning that timecode metadata is ignored if an explicit
timecode track is present.
2. There is another place where the src_track field is used even when a
timecode track doesn't have a src_track: When writing the moov tag
(lines 4156-4166). This will probably also need to be fixed, but it is
not dangerous.
3. There is btw no validation that a track with "tmcd" tag is a data
stream.
libavformat/movenc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 2006fcee4b..265465f97b 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2901,7 +2901,7 @@ static int mov_write_minf_tag(AVFormatContext *s,
AVIOContext *pb, MOVMuxContext
static int64_t calc_pts_duration(MOVMuxContext *mov, MOVTrack *track)
{
-if (track->tag == MKTAG('t','m','c','d')) {
+if (track->tag == MKTAG('t','m','c','d') && mov->nb_meta_tmcd) {
// tmcd tracks gets track_duration set in mov_write_moov_tag from
// another track's duration, while the end_pts may be left at zero.
This fix in itself is probably good and safe, so LGTM.
// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/2] avformat/rtsp: fix infinite loop with udp transport
Hi,
On Thu, 1 Oct 2020, Andriy Gelman wrote:
On Wed, 30. Sep 12:41, Martin Storsjö wrote:
Hi,
On Sun, 27 Sep 2020, Zhao Zhili wrote:
> Fix #8840.
>
> Steps to reproduce:
> 1. sender:
> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp
rtsp://localhost:12345/live.sdp
> 2. receiver:
> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i
rtsp://localhost:12345/live.sdp -c copy test.mp4
> ---
> v3: mention the ticket.
>
> libavformat/rtsp.c| 2 ++
> libavformat/rtsp.h| 1 +
> libavformat/rtspdec.c | 2 +-
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 5d8491b74b..597413803f 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -2051,6 +2051,8 @@ static int udp_read_packet(AVFormatContext *s,
RTSPStream **prtsp_st,
> if ((ret = parse_rtsp_message(s)) < 0) {
> return ret;
> }
> +if (rt->state == RTSP_STATE_TEARDOWN)
> +return AVERROR_EOF;
> }
> #endif
> } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
> diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
> index 54a9a30c16..481cc0c3ce 100644
> --- a/libavformat/rtsp.h
> +++ b/libavformat/rtsp.h
> @@ -198,6 +198,7 @@ enum RTSPClientState {
> RTSP_STATE_STREAMING, /**< initialized and sending/receiving data */
> RTSP_STATE_PAUSED, /**< initialized, but not receiving data */
> RTSP_STATE_SEEKING, /**< initialized, requesting a seek */
> +RTSP_STATE_TEARDOWN,/**< initialized, in teardown state */
> };
>
> /**
> diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
> index dfa29913bf..ec786a469a 100644
> --- a/libavformat/rtspdec.c
> +++ b/libavformat/rtspdec.c
> @@ -494,7 +494,7 @@ int ff_rtsp_parse_streaming_commands(AVFormatContext *s)
> "Public: ANNOUNCE, PAUSE, SETUP, TEARDOWN, "
> "RECORD\r\n", request.seq);
> } else if (methodcode == TEARDOWN) {
> -rt->state = RTSP_STATE_IDLE;
> +rt->state = RTSP_STATE_TEARDOWN;
> ret = rtsp_send_reply(s, RTSP_STATUS_OK, NULL , request.seq);
> }
> return ret;
> --
> 2.17.1
Martin, thanks for looking over the patch.
I think this approach to fixing it, adding a new state, is a bit of
overkill. This usecase actually used to work originally, but I bisected it
and it broke in f6161fccf8c5720ceac1ed1df8ba60ff8fed69f5. So with that in
mind, it's pretty straightforward to fix it by retaining the original
behaviour from before that commit. I'll send an alternative patch that does
that.
I made the suggestion to add TEARDOWN state because I thought it's safer than
relying on the idle state, and it made the code more readable.
Yeah, it's not a bad idea per se - but adding more states is generally
more risky (more codepaths that might need to take it into account, etc).
And in this case, as it's a regression, it's easier to fix it as a
specific fix for that breakage.
Looking at your patch, I think it's a clean/elegant solution, and also looks
good to me.
Great, thanks! So if Zhao also is ok with it, I'd push that one, and patch
2/2 from Zhao's set.
// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] opusdec: do not fail when LBRR frames are present
Quoting Lynne (2020-09-11 20:53:16)
> On 11/09/2020 19:37, Anton Khirnov wrote:
> > Decode and discard them.
> >
> > Fixes ticket 4641.
> > ---
> > libavcodec/opus_silk.c | 28
> > libavcodec/opustab.c | 3 +++
> > libavcodec/opustab.h | 3 +++
> > 3 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/libavcodec/opus_silk.c b/libavcodec/opus_silk.c
> > index 2fcbf3b9d3..c683b25a20 100644
> > --- a/libavcodec/opus_silk.c
> > +++ b/libavcodec/opus_silk.c
> > @@ -506,7 +506,8 @@ static inline void silk_decode_excitation(SilkContext
> > *s, OpusRangeCoder *rc,
> > #define LTP_ORDER 5
> >
> > static void silk_decode_frame(SilkContext *s, OpusRangeCoder *rc,
> > - int frame_num, int channel, int
> > coded_channels, int active, int active1)
> > + int frame_num, int channel, int
> > coded_channels,
> > + int active, int active1, int redundant)
> > {
> > /* per frame */
> > int voiced; // combines with active to indicate inactive,
> > active, or active+voiced
> > @@ -665,8 +666,8 @@ static void silk_decode_frame(SilkContext *s,
> > OpusRangeCoder *rc,
> > silk_decode_excitation(s, rc, residual + SILK_MAX_LAG, qoffset_high,
> > active, voiced);
> >
> > -/* skip synthesising the side channel if we want mono-only */
> > -if (s->output_channels == channel)
> > +/* skip synthesising the output if we do not need it */
> > +if (s->output_channels == channel || redundant)
> > return;
>
> Maybe add a small TODO in the comment to actually implement error recovery?
>
>
> >
> > /* generate the output signal */
> > @@ -814,15 +815,26 @@ int ff_silk_decode_superframe(SilkContext *s,
> > OpusRangeCoder *rc,
> > active[i][j] = ff_opus_rc_dec_log(rc, 1);
> >
> > redundancy[i] = ff_opus_rc_dec_log(rc, 1);
> > -if (redundancy[i]) {
> > -avpriv_report_missing_feature(s->avctx, "LBRR frames");
> > -return AVERROR_PATCHWELCOME;
> > -}
> > }
> >
> > +/* read the per-frame LBRR flags */
> > +for (i = 0; i < coded_channels; i++)
> > +if (redundancy[i] && duration_ms > 20) {
> > +redundancy[i] = ff_opus_rc_dec_cdf(rc, duration_ms == 40 ?
> > +
> > ff_silk_model_lbrr_flags_40 : ff_silk_model_lbrr_flags_60);
> > +}
> > +
> > +/* decode the LBRR frames */
> > +for (i = 0; i < nb_frames; i++)
> > +for (j = 0; j < coded_channels; j++)
> > +if (redundancy[j] & (1 << i)) {
> > +int active1 = (j == 0 && !(redundancy[1] & (1 << i))) ? 0
> > : 1;
> > +silk_decode_frame(s, rc, i, j, coded_channels, 1, active1,
> > 1);
> > +}
>
> nit: add brackets to the nb_frames loop
>
> Apart from that LGTM.
Applied comments and pushed
--
Anton Khirnov
___
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] avformat/libsrt: print streamid at client
Print the SRT streamid at the client. Logged to the context and changed info to verbose. Signed-off-by: raghavendra --- libavformat/libsrt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c index ee64cb82f7..b6b50302b7 100644 --- a/libavformat/libsrt.c +++ b/libavformat/libsrt.c @@ -364,7 +364,7 @@ static void libsrt_dump_streamid(URLContext *h, int fd) char streamid[512]; int optlen = sizeof(streamid); if(!libsrt_getsockopt(h, fd, SRTO_STREAMID, "SRTO_STREAMID", streamid, )) -av_log(NULL, AV_LOG_INFO, "srt_streamid : %s\n", streamid); +av_log(h, AV_LOG_VERBOSE, "srt_streamid : %s\n", streamid); } static int libsrt_setup(URLContext *h, const char *uri, int flags) -- 2.25.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] Example filter that process video and audio
On Sun, Sep 27, 2020 at 14:03:52 +0100, da...@3adesign.co.uk wrote: > Was looking to create a filter to process video and audio in a filter graph, > can find examples in movie or a source that has both video and audio but not > anything that takes both, any pointers to something to use as a reference? Are you looking to process both vidoe and audio inputs in *one* filter, creating something which needs both inputs? This hasn't been done yet, but I'm not sure it's impossible. Do note that there are filters that convert audio to video, and vice versa: $ ffmpeg -filters | grep -E 'V->A|A->V' 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] [PATCH] avformat/libsrt: print streamid at client
raghavendra (12020-10-01):
> Print the SRT streamid at the client.
>
> Signed-off-by: raghavendra
> ---
> libavformat/libsrt.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
> index a490a386e6..ee64cb82f7 100644
> --- a/libavformat/libsrt.c
> +++ b/libavformat/libsrt.c
> @@ -359,6 +359,13 @@ static int libsrt_set_options_pre(URLContext *h, int fd)
> return 0;
> }
>
> +static void libsrt_dump_streamid(URLContext *h, int fd)
> +{
> +char streamid[512];
> +int optlen = sizeof(streamid);
> +if(!libsrt_getsockopt(h, fd, SRTO_STREAMID, "SRTO_STREAMID", streamid,
> ))
> +av_log(NULL, AV_LOG_INFO, "srt_streamid : %s\n", streamid);
Do not log to NULL. You have a context.
Is this information relevant for a normal user? If not, info → verbose.
> +}
>
> static int libsrt_setup(URLContext *h, const char *uri, int flags)
> {
> @@ -442,6 +449,8 @@ static int libsrt_setup(URLContext *h, const char *uri,
> int flags)
> goto fail1;
> listen_fd = fd;
> fd = ret;
> +// dump srt streamid at client
> +libsrt_dump_streamid(h, fd);
> } else {
> if (s->mode == SRT_MODE_RENDEZVOUS) {
> ret = srt_bind(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
Regards,
--
Nicolas George
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".
[FFmpeg-devel] [PATCH] avformat/libsrt: print streamid at client
Print the SRT streamid at the client.
Signed-off-by: raghavendra
---
libavformat/libsrt.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
index a490a386e6..ee64cb82f7 100644
--- a/libavformat/libsrt.c
+++ b/libavformat/libsrt.c
@@ -359,6 +359,13 @@ static int libsrt_set_options_pre(URLContext *h, int fd)
return 0;
}
+static void libsrt_dump_streamid(URLContext *h, int fd)
+{
+char streamid[512];
+int optlen = sizeof(streamid);
+if(!libsrt_getsockopt(h, fd, SRTO_STREAMID, "SRTO_STREAMID", streamid,
))
+av_log(NULL, AV_LOG_INFO, "srt_streamid : %s\n", streamid);
+}
static int libsrt_setup(URLContext *h, const char *uri, int flags)
{
@@ -442,6 +449,8 @@ static int libsrt_setup(URLContext *h, const char *uri, int
flags)
goto fail1;
listen_fd = fd;
fd = ret;
+// dump srt streamid at client
+libsrt_dump_streamid(h, fd);
} else {
if (s->mode == SRT_MODE_RENDEZVOUS) {
ret = srt_bind(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
--
2.25.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".
