| Message ID | 20200716145450.66648-1-epirat07@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] adtsenc: Add ability to specify MPEG ID | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Thu, 16 Jul 2020 at 16:25, Marvin Scholz <epirat07@gmail.com> wrote: > --- > > This stills lacks docs, as I first want an overall opinion on this > approach or possible other solutions I should choose for this. > What uses this? Kieran
On 16 Jul 2020, at 17:27, Kieran Kunhya wrote: > On Thu, 16 Jul 2020 at 16:25, Marvin Scholz <epirat07@gmail.com> > wrote: > >> --- >> >> This stills lacks docs, as I first want an overall opinion on this >> approach or possible other solutions I should choose for this. >> > > What uses this? It's needed when concatenating streams with existing ADTS streams that have the same parameters, except for the differing MPEG ID. Such a stream would be invalid if the MPEG IDs are different as then the fixed header part of the ADTS header changes mid-stream, which is not allowed. FFmpeg does not care and still works fine for such broken streams but some other players do and will break trying to play such streams. I could not find any reason for the choice of used MPEG ID in FFmpeg, and in the specifications it was not clear to me either if there is even one "right" one to be used, so the best way to solve this seemed to allow to choose it. > > Kieran
On 16 Jul 2020, at 18:11, Hendrik Leppkes wrote: > On Thu, Jul 16, 2020 at 5:28 PM Kieran Kunhya <kierank@obe.tv> wrote: >> >> On Thu, 16 Jul 2020 at 16:25, Marvin Scholz <epirat07@gmail.com> wrote: >> >>> --- >>> >>> This stills lacks docs, as I first want an overall opinion on this >>> approach or possible other solutions I should choose for this. >>> >> >> What uses this? >> > > In my experience some older devices also require the type set to MPEG2. > > For the patch itself - > Its generally a good idea to be able to specify it, but i'm not sure > about the option chosen. Its a single bit value for swapping between > mpeg2 and mpeg4, maybe it should just be boolean? > Sure, thats of course possible too. I have no strong opinions about that at all. (It's just mostly that it could not come up with a good idea for a boolean option name for this) > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/adtsenc.c b/libavformat/adtsenc.c index 9e285752eb..601d923526 100644 --- a/libavformat/adtsenc.c +++ b/libavformat/adtsenc.c @@ -40,6 +40,7 @@ typedef struct ADTSContext { int pce_size; int apetag; int id3v2tag; + int mpeg_id; uint8_t pce_data[MAX_PCE_SIZE]; } ADTSContext; @@ -136,7 +137,7 @@ static int adts_write_frame_header(ADTSContext *ctx, /* adts_fixed_header */ put_bits(&pb, 12, 0xfff); /* syncword */ - put_bits(&pb, 1, 0); /* ID */ + put_bits(&pb, 1, ctx->mpeg_id); /* ID */ put_bits(&pb, 2, 0); /* layer */ put_bits(&pb, 1, 1); /* protection_absent */ put_bits(&pb, 2, ctx->objecttype); /* profile_objecttype */ @@ -214,6 +215,9 @@ static int adts_write_trailer(AVFormatContext *s) static const AVOption options[] = { { "write_id3v2", "Enable ID3v2 tag writing", OFFSET(id3v2tag), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, ENC}, { "write_apetag", "Enable APE tag writing", OFFSET(apetag), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, ENC}, + { "mpeg_id", "Select which MPEG ID to write", OFFSET(mpeg_id), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, ENC, "mpeg_id"}, + { "MPEG4", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, ENC, "mpeg_id" }, + { "MPEG2", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, ENC, "mpeg_id" }, { NULL }, };