diff mbox series

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

Message ID HE1PR0301MB2154DFEA4856F8C5CF45179C8F799@HE1PR0301MB2154.eurprd03.prod.outlook.com
State Accepted
Headers show
Series [FFmpeg-devel] avformat/mpegtsenc: Preserve disposition in the absence of language
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 3, 2021, 5:19 a.m. UTC
Implements ticket #9113.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/mpegtsenc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

stephen douglas April 3, 2021, 6:55 a.m. UTC | #1
ISO 639 language descriptors used by DVB are the 3 charecter variants hence 
const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;should beconst char *language = lang && strlen(lang->value) > 3 ? lang->value : default_language;
   On Saturday, 3 April 2021, 06:53:20 BST, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:  
 
 Implements ticket #9113.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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) {
                    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) {
Andreas Rheinhardt April 3, 2021, 7:15 a.m. UTC | #2
stephen douglas:
>  ISO 639 language descriptors used by DVB are the 3 charecter variants hence 
> const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;should beconst char *language = lang && strlen(lang->value) > 3 ? lang->value : default_language;

1. Stop top-posting.
2. Are you even aware that strlen does not include the trailing \0?

>    On Saturday, 3 April 2021, 06:53:20 BST, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:  
>  
>  Implements ticket #9113.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
Marton Balint April 4, 2021, 1:20 p.m. UTC | #3
On Sat, 3 Apr 2021, Andreas Rheinhardt wrote:

> Implements ticket #9113.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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; ...)

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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt April 4, 2021, 4:13 p.m. UTC | #4
Marton Balint:
> 
> 
> On Sat, 3 Apr 2021, Andreas Rheinhardt wrote:
> 
>> Implements ticket #9113.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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) {

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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".
Marton Balint April 4, 2021, 4:51 p.m. UTC | #5
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@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@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@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-request@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-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

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) {
                     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) {