diff mbox series

[FFmpeg-devel] adtsenc: Add ability to specify MPEG ID

Message ID 20200716145450.66648-1-epirat07@gmail.com
State New
Headers show
Series [FFmpeg-devel] adtsenc: Add ability to specify MPEG ID
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Marvin Scholz July 16, 2020, 2:54 p.m. UTC
---

This stills lacks docs, as I first want an overall opinion on this
approach or possible other solutions I should choose for this.

---
 libavformat/adtsenc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Kieran Kunhya July 16, 2020, 3:27 p.m. UTC | #1
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
Marvin Scholz July 16, 2020, 3:31 p.m. UTC | #2
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
Marvin Scholz July 16, 2020, 7:59 p.m. UTC | #3
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 mbox series

Patch

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 },
 };