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

[Federico Simoncelli]

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

and it will generate non-representable timebases:

1/48 = .020833...
1/44.1 = .0226757

> and the frame_size either should be "redefined" to mean samples
> across all channels so that 8bit samples with 3 channels and a
> frame size of 1024 means 3072 bytes

I thought that producing a warning was good enough (consider also the
answer in the next paragraph).
Are you sure you want break the backward compatibility of the
frame_size parameter?

> cutting frames between channels is quiet bad so a frame size
> in bytes is problematic but if its kept in bytes then a default of
> 3360 should avoid the fractional durations with the changed timebase

I think that my patch handles the frames cut between channels case. At
the denominator there's "channels":

frame_duration = (frame_size * 1000000 * 8) /
                 (sample_rate * channels * bits_per_sample)

-- 
Federico