[FFmpeg-devel] [PATCH] avformat/mpegtsenc: Preserve disposition in the absence of language

[Marton Balint]

On Sun, 4 Apr 2021, Andreas Rheinhardt wrote:

> Marton Balint:
>> 
>> 
>> On Sat, 3 Apr 2021, Andreas Rheinhardt wrote:
>> 
>>> Implements ticket #9113.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>> ---
>>> libavformat/mpegtsenc.c | 18 +++++++++---------
>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
>>> index 35c835c484..dbd3bb148a 100644
>>> --- a/libavformat/mpegtsenc.c
>>> +++ b/libavformat/mpegtsenc.c
>>> @@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s,
>>> MpegTSService *service)
>>> Â Â Â Â Â Â Â  AVStream *st = s->streams[i];
>>> Â Â Â Â Â Â Â  MpegTSWriteStream *ts_st = st->priv_data;
>>> Â Â Â Â Â Â Â  AVDictionaryEntry *lang = av_dict_get(st->metadata,
>>> "language", NULL, 0);
>>> +Â Â Â Â Â Â Â  const char default_language[] = "und";
>>> +Â Â Â Â Â Â Â  const char *language = lang && strlen(lang->value) >= 3 ?
>>> lang->value : default_language;
>>> Â Â Â Â Â Â Â  enum AVCodecID codec_id = st->codecpar->codec_id;
>>>
>>> Â Â Â Â Â Â Â  if (s->nb_programs) {
>>> @@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s,
>>> MpegTSService *service)
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  }
>>> Â Â Â Â Â Â Â Â Â Â Â  }
>>>
>>> -Â Â Â Â Â Â Â Â Â Â Â  if (lang) {
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  char *p;
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  char *next = lang->value;
>>> +Â Â Â Â Â Â Â Â Â Â Â  if (language != default_language ||
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  st->disposition & (AV_DISPOSITION_CLEAN_EFFECTSÂ Â Â  |
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  AV_DISPOSITION_HEARING_IMPAIRED |
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  AV_DISPOSITION_VISUAL_IMPAIRED)) {
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  const char *p;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  const char *next = language;
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  uint8_t *len_ptr;
>>>
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  *q++Â Â Â Â  = ISO_639_LANGUAGE_DESCRIPTOR;
>>>                 len_ptr  = q++;
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  *len_ptr = 0;
>>>
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  for (p = lang->value; next && *len_ptr < 255 / 4 * 4;
>>> p = next + 1) {
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  for (p = next; next && *len_ptr < 255 / 4 * 4; p =
>>> next + 1) {
>> 
>> Maybe it would make the code more readable to do both initializations in
>> the for() statement, e.g.: for (p = next = language; ...)
>> 
>
> The reason I didn't do so is that it would make the line above 80
> characters. So the alternatives to my current patch would be:
>
>                for (p = next = language; next && *len_ptr < 255 / 4 * 4;
>                     p = next + 1) {

I like this the most wrapped to a single line. I don't think adhereing to 
the 80 char limit makes the code more readable here, so I'd simply ignore 
it.

Regards,
Marton

>
> or
>
>                for (const char *p = language, *next = p;
>                     next && *len_ptr < 255 / 4 * 4; p = next + 1) {
>
> or factoring out writing the ISO 639 language descriptor into a function
> of its own or just ignoring the 80 chars line limit.
> What do you prefer?
>
>> LGTM otherwise.
>> 
>> Thanks,
>> Marton
>> 
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  if (q - data > SECTION_LENGTH - 4) {
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  err = 1;
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  break;
>>> @@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
>>> MpegTSService *service)
>>> Â Â Â Â Â Â Â Â Â Â Â  }
>>> Â Â Â Â Â Â Â Â Â Â Â  break;
>>> Â Â Â Â Â Â Â  case AVMEDIA_TYPE_SUBTITLE:
>>> -Â Â Â Â Â Â Â  {
>>> -Â Â Â Â Â Â Â Â Â Â  const char default_language[] = "und";
>>> -Â Â Â Â Â Â Â Â Â Â  const char *language = lang && strlen(lang->value) >= 3 ?
>>> lang->value : default_language;
>>> -
>>> Â Â Â Â Â Â Â Â Â Â  if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â  uint8_t *len_ptr;
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â  int extradata_copied = 0;
>>> @@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
>>> MpegTSService *service)
>>>
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â  *len_ptr = q - len_ptr - 1;
>>> Â Â Â Â Â Â Â Â Â Â Â  }
>>> -Â Â Â Â Â Â Â  }
>>> Â Â Â Â Â Â Â  break;
>>> Â Â Â Â Â Â Â  case AVMEDIA_TYPE_VIDEO:
>>> Â Â Â Â Â Â Â Â Â Â Â  if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
>>> --Â 
>>> 2.27.0
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".