[FFmpeg-devel] [PATCH] pulse: set default frame_size to 4608

[Federico Simoncelli]

On Fri, Jan 3, 2014 at 6:41 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Jan 03, 2014 at 04:55:33PM +0100, Federico Simoncelli wrote:
>> On Fri, Jan 3, 2014 at 3:44 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Fri, Jan 03, 2014 at 01:21:41AM +0100, Federico Simoncelli wrote:
>> >> On Thu, Jan 2, 2014 at 11:26 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Thu, Jan 02, 2014 at 04:36:45PM +0100, Federico Simoncelli wrote:
>> >> >> Given the current defaults (channels = 2, sample_rate = 48000) the
>> >> >> frame_size is changed to 4608 in order to obtain whole numbers for
>> >> >> frame_duration (both 16 and 24 bits_per_sample).
>> >> >>
>> >> >>  frame_duration = (frame_size * 1000000 * 8) /
>> >> >>                   (sample_rate * channels * bits_per_sample)
>> >> >>
>> >> >> A message has been added to warn the user when the frame duration
>> >> >> is not an integer.
>> >> >
>> >> > the timebase should be changed to a multiple of the sample rate
>> >> > see the avpriv_set_pts_info() call
>> >> > that avoids the problem of the durations being not exactly
>> >> > representable
>> >>
>> >> If I understand correctly what you're suggesting is:
>> >>
>> >> avpriv_set_pts_info(st, 64, 1, pd->sample_rate);
>> >
>> > yes
>> >
>> >> and it will generate non-representable timebases:
>> >>
>> >> 1/48 = .020833...
>> >> 1/44.1 = .0226757
>> >
>> > rational numbers are quite representable, and really 1/1000000 is
>> > a rational number in no way different than 1/48 or 1/48000 is
>> > also if you dont like fractions you can just change the base of
>> > your representation
>> >
>> > ...
>> >
>> > so really the non representability is just an artifact of humans
>> > having 10 fingers
>>
>> Yes, that's clear, what I meant is that there was 10-fingered code
>> around somewhere since I was getting pts warnings after changing the
>> timebase.
>> Instead I pinpointed this to be something related to the mp3 encoder
>> (I don't know what yet).
>
> how can i reproduce that issue ?
> can you share the work in progress patch ?

I just found out that this may be related to the side data:

[mp3 @ 0x191e320] skip 1105 samples due to side data
[mp3 @ 0x191e320] skip 1105/1152 samples

> frame_pts = 1105, frame_ts = 0.023021, base_ts = 0.000021
> frame_pts = 1152, frame_ts = 0.024000, base_ts = 0.000021
> frame_pts = 2304, frame_ts = 0.048000, base_ts = 0.000021
> frame_pts = 3456, frame_ts = 0.072000, base_ts = 0.000021

There's one thing that is not clear to me, if the side data are frames
to be discarded, why is the pts adjusted?

1152 frames, 1105 are side data (47 remaining frames), now I'd expect
that when I process the first 47 valid frames the pts would be (in my
case) 0, otherwise (as the current code does) we'd be shifting them of
0.023s.
Am I missing something?

-- 
Federico