| Message ID | 20181025125917.31923-2-martin@martin.st |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 25, 2018 at 03:59:17PM +0300, Martin Storsjö wrote: > --- > libavformat/flv.h | 1 + > libavformat/flvdec.c | 21 +++++++++++++++++---- > 2 files changed, 18 insertions(+), 4 deletions(-) [...] > @@ -1290,6 +1302,7 @@ static const AVOption options[] = { > { "flv_full_metadata", "Dump full metadata of the onMetadata", OFFSET(dump_full_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > { "flv_ignore_prevtag", "Ignore the Size of previous tag", OFFSET(trust_datasize), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > { "missing_streams", "", OFFSET(missing_streams), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY }, > + { "export_opaque_meta", "", OFFSET(export_opaque_meta), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > { NULL } I think this together with doc/demuxers.texi (which doesnt document this) is not enough to use this option by a user also why is this conditional ? is there a disadvantage of always exporting this ? [...]
On Sat, 27 Oct 2018, Michael Niedermayer wrote: > On Thu, Oct 25, 2018 at 03:59:17PM +0300, Martin Storsjö wrote: >> --- >> libavformat/flv.h | 1 + >> libavformat/flvdec.c | 21 +++++++++++++++++---- >> 2 files changed, 18 insertions(+), 4 deletions(-) > > [...] >> @@ -1290,6 +1302,7 @@ static const AVOption options[] = { >> { "flv_full_metadata", "Dump full metadata of the onMetadata", OFFSET(dump_full_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >> { "flv_ignore_prevtag", "Ignore the Size of previous tag", OFFSET(trust_datasize), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >> { "missing_streams", "", OFFSET(missing_streams), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY }, >> + { "export_opaque_meta", "", OFFSET(export_opaque_meta), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >> { NULL } > > I think this together with doc/demuxers.texi (which doesnt document this) > is not enough to use this option by a user Oh right, I had forgotten to actually write something here. > also why is this conditional ? is there a disadvantage of always > exporting this ? Not sure - I thought it'd be less behaviour change and less risk of potentially confusing packets for unsuspecting users by not doing it by default. But as any normal flv stream doesn't contain any such packets, it might be fine to just expose them all the time. // Martin
On Sat, Oct 27, 2018 at 09:22:18PM +0300, Martin Storsjö wrote: > On Sat, 27 Oct 2018, Michael Niedermayer wrote: > > >On Thu, Oct 25, 2018 at 03:59:17PM +0300, Martin Storsjö wrote: > >>--- > >> libavformat/flv.h | 1 + > >> libavformat/flvdec.c | 21 +++++++++++++++++---- > >> 2 files changed, 18 insertions(+), 4 deletions(-) > > > >[...] > >>@@ -1290,6 +1302,7 @@ static const AVOption options[] = { > >> { "flv_full_metadata", "Dump full metadata of the onMetadata", OFFSET(dump_full_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > >> { "flv_ignore_prevtag", "Ignore the Size of previous tag", OFFSET(trust_datasize), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > >> { "missing_streams", "", OFFSET(missing_streams), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY }, > >>+ { "export_opaque_meta", "", OFFSET(export_opaque_meta), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, > >> { NULL } > > > >I think this together with doc/demuxers.texi (which doesnt document this) > >is not enough to use this option by a user > > Oh right, I had forgotten to actually write something here. > > >also why is this conditional ? is there a disadvantage of always > >exporting this ? > > Not sure - I thought it'd be less behaviour change and less risk of > potentially confusing packets for unsuspecting users by not doing it by > default. But as any normal flv stream doesn't contain any such packets, it > might be fine to just expose them all the time. I dont know enough about these to have an oppinion ... but I just realized another aspect. How do these packets interact with flvenc ? Should they be preserved by default ? because if so then they would need to be exported by default [...]
On Sun, 28 Oct 2018, Michael Niedermayer wrote: > On Sat, Oct 27, 2018 at 09:22:18PM +0300, Martin Storsjö wrote: >> On Sat, 27 Oct 2018, Michael Niedermayer wrote: >> >>> On Thu, Oct 25, 2018 at 03:59:17PM +0300, Martin Storsjö wrote: >>>> --- >>>> libavformat/flv.h | 1 + >>>> libavformat/flvdec.c | 21 +++++++++++++++++---- >>>> 2 files changed, 18 insertions(+), 4 deletions(-) >>> >>> [...] >>>> @@ -1290,6 +1302,7 @@ static const AVOption options[] = { >>>> { "flv_full_metadata", "Dump full metadata of the onMetadata", OFFSET(dump_full_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >>>> { "flv_ignore_prevtag", "Ignore the Size of previous tag", OFFSET(trust_datasize), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >>>> { "missing_streams", "", OFFSET(missing_streams), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY }, >>>> + { "export_opaque_meta", "", OFFSET(export_opaque_meta), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, >>>> { NULL } >>> >>> I think this together with doc/demuxers.texi (which doesnt document this) >>> is not enough to use this option by a user >> >> Oh right, I had forgotten to actually write something here. >> >>> also why is this conditional ? is there a disadvantage of always >>> exporting this ? >> >> Not sure - I thought it'd be less behaviour change and less risk of >> potentially confusing packets for unsuspecting users by not doing it by >> default. But as any normal flv stream doesn't contain any such packets, it >> might be fine to just expose them all the time. > > I dont know enough about these to have an oppinion ... > > but I just realized another aspect. How do these packets interact with > flvenc ? > Should they be preserved by default ? because if so then they would need > to be exported by default I guess it depends on what the packets actually are - as it can be anything, it's pretty much up to the application what treatment they want for them. flvenc right now does write them out properly afaik (a data track with codec type AV_CODEC_ID_NONE gets copied straight through into FLV_TAG_TYPE_META packets). I guess the sensible default would be to copy them, so I guess I'll amend the patch to always export them. // Martin
diff --git a/libavformat/flv.h b/libavformat/flv.h index 3aabb3adc9..3571b90279 100644 --- a/libavformat/flv.h +++ b/libavformat/flv.h @@ -66,6 +66,7 @@ enum { FLV_STREAM_TYPE_VIDEO, FLV_STREAM_TYPE_AUDIO, FLV_STREAM_TYPE_SUBTITLE, + FLV_STREAM_TYPE_DATA, FLV_STREAM_TYPE_NB, }; diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c index ffc975f15d..b4e4ff3907 100644 --- a/libavformat/flvdec.c +++ b/libavformat/flvdec.c @@ -72,6 +72,8 @@ typedef struct FLVContext { int64_t *keyframe_filepositions; int missing_streams; AVRational framerate; + + int export_opaque_meta; } FLVContext; static int probe(AVProbeData *p, int live) @@ -143,7 +145,9 @@ static AVStream *create_stream(AVFormatContext *s, int codec_type) st->codecpar->codec_type = codec_type; if (s->nb_streams>=3 ||( s->nb_streams==2 && s->streams[0]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE - && s->streams[1]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE)) + && s->streams[1]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE + && s->streams[0]->codecpar->codec_type != AVMEDIA_TYPE_DATA + && s->streams[1]->codecpar->codec_type != AVMEDIA_TYPE_DATA)) s->ctx_flags &= ~AVFMTCTX_NOHEADER; if (codec_type == AVMEDIA_TYPE_AUDIO) { st->codecpar->bit_rate = flv->audio_bit_rate; @@ -1001,7 +1005,7 @@ retry: int type; meta_pos = avio_tell(s->pb); type = flv_read_metabody(s, next); - if (type == 0 && dts == 0 || type < 0 || type == TYPE_UNKNOWN) { + if (type == 0 && dts == 0 || type < 0 || (type == TYPE_UNKNOWN && !flv->export_opaque_meta)) { if (type < 0 && flv->validate_count && flv->validate_index[0].pos > next && flv->validate_index[0].pos - 4 < next @@ -1015,6 +1019,8 @@ retry: return flv_data_packet(s, pkt, dts, next); } else if (type == TYPE_ONCAPTION) { return flv_data_packet(s, pkt, dts, next); + } else if (type == TYPE_UNKNOWN) { + stream_type = FLV_STREAM_TYPE_DATA; } avio_seek(s->pb, meta_pos, SEEK_SET); } @@ -1054,10 +1060,13 @@ skip: } else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) { if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) break; + } else if (stream_type == FLV_STREAM_TYPE_DATA) { + if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) + break; } } if (i == s->nb_streams) { - static const enum AVMediaType stream_types[] = {AVMEDIA_TYPE_VIDEO, AVMEDIA_TYPE_AUDIO, AVMEDIA_TYPE_SUBTITLE}; + static const enum AVMediaType stream_types[] = {AVMEDIA_TYPE_VIDEO, AVMEDIA_TYPE_AUDIO, AVMEDIA_TYPE_SUBTITLE, AVMEDIA_TYPE_DATA}; st = create_stream(s, stream_types[stream_type]); if (!st) return AVERROR(ENOMEM); @@ -1153,6 +1162,8 @@ retry_duration: size -= ret; } else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) { st->codecpar->codec_id = AV_CODEC_ID_TEXT; + } else if (stream_type == FLV_STREAM_TYPE_DATA) { + st->codecpar->codec_id = AV_CODEC_ID_NONE; // Opaque AMF data } if (st->codecpar->codec_id == AV_CODEC_ID_AAC || @@ -1253,7 +1264,8 @@ retry_duration: if ( stream_type == FLV_STREAM_TYPE_AUDIO || ((flags & FLV_VIDEO_FRAMETYPE_MASK) == FLV_FRAME_KEY) || - stream_type == FLV_STREAM_TYPE_SUBTITLE) + stream_type == FLV_STREAM_TYPE_SUBTITLE || + stream_type == FLV_STREAM_TYPE_DATA) pkt->flags |= AV_PKT_FLAG_KEY; leave: @@ -1290,6 +1302,7 @@ static const AVOption options[] = { { "flv_full_metadata", "Dump full metadata of the onMetadata", OFFSET(dump_full_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, { "flv_ignore_prevtag", "Ignore the Size of previous tag", OFFSET(trust_datasize), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, { "missing_streams", "", OFFSET(missing_streams), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY }, + { "export_opaque_meta", "", OFFSET(export_opaque_meta), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD }, { NULL } };