[FFmpeg-devel] [PATCH v4 0/4] add ARIB caption decoder using libaribcaption

[TADANO Tokumei]

Thanks for the comments.
I'll update the patch set.

... and comment inline:

On 2022/07/02 1:08, Mao Hata wrote:
> In aribcaption_close() (libaribcaption.c):
> ```
> if (ctx->renderer) {
>      aribcc_renderer_free(ctx->renderer);
>      ctx->decoder = NULL;
> }
> ```
> This should be `ctx->renderer = NULL;`.
> In set_ass_header() (libaribcaption.c):
> ```
> if (fonts && *fonts) {
>      font_name = av_get_token(&fonts, ",");
>      if (!font_name)
>          return AVERROR(ENOMEM);
> } else
>      font_name = av_strdup(DEFAULT_FONT_FAMILY);
> ```
> You should reposition `if (!font_name) return AVERROR(ENOMEM);`, because av_strdup() can also return NULL.
> On some "return"s in aribcaption_init(), I think you have forgotten aribcaption_close(avctx).
> 
> I have tested the features of this patch using some sample files. As you recommend, the files under https://streams.videolan.org/streams/ts/ARIB/ are good samples. In particular, https://streams.videolan.org/streams/ts/ARIB/japan/osaka_15.ts uses various subtitle expressions, I like it.
> 
> # ASS formatted text
> $ ffmpeg -fix_sub_duration -i osaka_15.ts osaka_15.ass
> 
> I think the created ASS by the above command is much better than that of aribb24. (Erase timing for each subtitle of aribb24 was very inaccurate.) Then, it's trivial, but is there any reason to set the default "Alignment" to 2 and always override it with "{\an7}"? I think the default "Alignment" should be 7.

The default alignment (ASS_DEFAULT_ALIGNMENT) is defined in libavcodec/ass.h
It is common for all codecs and I can't modify it for ARIB caption only.

I tried to change it in ass header by set_ass_header() function, however,
most players don't interpret preset ass header and follow re-generated their own
ass header. Thus, I always set "{\an7}" on each line.

> # Overlay subtitles on the video
> $ ffmpeg -sub_type bitmap -i osaka_15.ts -filter_complex "[0:v][0:s]overlay" -s 1280x720 osaka_15.mp4
> 
> The bitmap feature is a cool one. Its output quality is the same as the subtitles I usually watch on Japanese TV devices. I tested the feature with many personal files, no particular problem found.
> 
> Mao Hata
> 
> On 2022/06/24 19:06, TADANO Tokumei wrote:
>> 3rd ping!
>>
>> Are there any other objections to this patch set?
>> If not, would someone push it to the repository?
>>
>> A comment inline:
>>
>> On 2022/06/17 0:30, TADANO Tokumei wrote:
>>>
>>> On 2022/06/16 22:40, Soft Works wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>>> TADANO Tokumei
>>>>> Sent: Thursday, June 16, 2022 3:23 PM
>>>>> To: ffmpeg-devel at ffmpeg.org
>>>>> Subject: Re: [FFmpeg-devel] [PATCH v4 0/4] add ARIB caption decoder
>>>>> using libaribcaption
>>>>>
>>>>> ping again!
>>>>>
>>>>> Are there any objections to this patch set?
>>>>> If not, would someone push it to the repository?
>>>>>
>>>>> On 2022/06/08 18:48, TADANO Tokumei wrote:
>>>>>> ping
>>>>>>
>>>>>> The patch set has been well tested by Japanese ISDB-related
>>>>> developers and works fine.
>>>>>> I think it already has good quality to merge.
>>>>>>
>>>>>> It requires external library like libaribb24, but the feature is
>>>>> disabled by default.
>>>>>> There is no impact to current code without `--enable-
>>>>> libaribcaption` option is
>>>>>> specified at configuration. The patch set provide better subtitle
>>>>> feature
>>>>>> for ISDB users than libaribb24 if enabled.
>>>>
>>>> What exactly is better than with the other ARIB decoder?
>>>>
>>>>
>>>>>> On 2022/05/30 23:55, TADANO Tokumei wrote:
>>>>>>> This patch set add another ARIB caption decoder using
>>>>> libaribcaption
>>>>>>> external library: https://github.com/xqq/libaribcaption
>>>>>>> The library decodes subtitles of ISDB-based TV broadcasting.
>>>>>>> It is not only for Japanese ARIB STD-B24 caption, but also for
>>>>>>> Brazilian ABNT NBR 15606-1 and Philippines version of ISDB-T.
>>>>>>>
>>>>>>> Unlike libaribb24, it supports 3 types of subtitle outputs:
>>>>>>> * text: plain text
>>>>>>> * ass: ASS formatted text
>>>>>>> * bitmap: bitmap image
>>>>
>>>> This will become obsolete with the introduction of subtitle
>>>> filtering (https://github.com/ffstaging/FFmpeg/pull/18).
>>
>> The subtitle filtering is not ready yet.
>> There is no reason to prevent to apply this patch set.
>> After the subtitle filtering will be ready, I can modify it to follow new API.
>>
>>>> Just recently, Traian has joined and contributed a filter
>>>> for converting text  subtitles to graphic subtitles
>>>> (https://github.com/softworkz/FFmpeg/pull/1), which fills
>>>> the last remaining gap for subtitle conversions.
>>>>
>>>>
>>>> Is this the only advantage over the existing ARIB caption decoder?
>>>
>>> The existing ARIB caption decoder (libaribb24) has lower reproducibility.
>>> For example, it lacks position information that is in original ARIB caption data.
>>> With this patch set, the intended caption is almost reproduced as ARIB standard
>>> states.
>>>
>>> Thanks,
>>> TADANO
>>>
>>>> Thanks,
>>>> sw
> _______________________________________________
> 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".