Re: [FFmpeg-devel] [PATCH] avformat/avcodec: Add DTS-UHD demuxer and parser, movenc support.

[Paul B Mahol <onemda@gmail.com>]

On Wed, Jun 14, 2023 at 8:06 AM Paul B Mahol <onemda@gmail.com> wrote:

>
>
> On Wed, Jun 14, 2023 at 8:01 AM Paul B Mahol <onemda@gmail.com> wrote:
>
>>
>>
>> On Wed, Jun 14, 2023 at 7:37 AM Paul B Mahol <onemda@gmail.com> wrote:
>>
>>>
>>>
>>> On Wed, Jun 14, 2023 at 2:00 AM Roy Funderburk <royffmpeg@funderburk.us>
>>> wrote:
>>>
>>>>
>>>> On 6/13/23 11:35 AM, Paul B Mahol wrote:
>>>> > Doing allocation in probe?
>>>> > Probing should be very fast.
>>>>
>>>> In line 143 of the avformat patch, memory allocation is removed from
>>>> the probe
>>>>
>>>> >>+int dtsuhd_frame(DTSUHD *h, const uint8_t *data, size_t data_bytes,
>>>> >>+                DTSUHDFrameInfo *fi, DTSUHDDescriptorInfo *di)
>>>> >>+{
>>>> >>+    gb = &h->gb;
>>>> >>+    init_get_bits(gb, data, data_bytes * 8);
>>>> > init_get_bits8, and check return code.
>>>>
>>>> In line 986 of the avcodec patch, changed to using init_get_bits8 and
>>>> added return code check.
>>>>
>>>> >>+        fi->sync = h->is_sync_frame;
>>>> >>+        fi->frame_bytes = h->frame_bytes;
>>>> >>+        fi->sample_rate = h->sample_rate;
>>>> >>+        fi->sample_count = (h->frame_duration * fi->sample_rate)
>>>> /(h->clock_rate * fraction);
>>>> >>+        fi->duration = (double)fi->sample_count / fi->sample_rate;
>>>> >Please  no double  type.
>>>> >Also make use of av_rescale.
>>>>
>>>> Around line 1108 of the avcodec patch, I found the duration was not
>>>> needed and removed it.
>>>>
>>>>
>>>> On 6/13/23 12:04 PM, Anton Khirnov wrote:
>>>> > Also do note that sharing structs across libraries opens you to
>>>> various
>>>> > compatibility questions [2]. It might be easier to sidestep them by
>>>> > having a function in libavcodec that accepts AVCodecParameters and
>>>> fills
>>>> > them according to the data, rather than pass codec-specific structs
>>>> > between libavformat and libavcodec.
>>>>
>>>> In line 1061 of the avcodec patch, changed the function to:
>>>>   int av_dtsuhd_frame(DTSUHD *h, const uint8_t *data, size_t data_bytes,
>>>>         AVCodecParameters *codecpar, uint8_t **udts, int *udts_size)
>>>>
>>>>
>>>>
>>> Usually libavcodec code does use get_vlc2() for variable length codes,
>>> instead of
>>> usage of show_bits/skip_bits/get_bits.
>>>
>>>
>> Also there is no reason to use int for elements in tables when max value
>> can be lower.
>> Current table reading/handling code should be completely rewritten to use
>> get_vlc2().
>> And tables split so length of codes use uint8_t type.
>>
>
> In parse_stream_params() function, there is skip_bits(gb,
> 36*get_bits1(gb))
>
> but skip_bits can read usually max 25 bits.
> Use skip_bits_long().
>

Also please remove from structs ':1' suffix and similar if that is not
really required to have exactly single bit max usage signaled per item in
struct.


>
>
>
>>
>>>
>>>
>>>
>>>>
>>>> Thank you for reviewing this,
>>>> -Roy_______________________________________________
>>>> 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".