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

[Mao Hata]

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.

# 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