[FFmpeg-devel] [PATCH 1/2] avutil: Add av_get_time_base{_q, }() and deprecate AV_TIME_BASE{_Q, }

[Derek Buitenhuis]

On 12/30/2013 1:58 AM, Michael Niedermayer wrote:
> On Sun, Dec 29, 2013 at 11:35:37PM +0000, Derek Buitenhuis wrote:
>> It was never a good idea to have tehse values as defines in the first place,
>> since they are internal values, and may change.
> 
> they start with a AV_ prefix and are in a public header, so the
> statement that they ARE "internal values, and may change" is
> certainly not correct.

Their own documentation begs to differ:

    /**
     * Internal time base represented as integer
     */

    [...]

    /**
     * Internal time base represented as fractional value
     */

Internal in the sense of "we use this time base internally", and not
"for internal use only". The user doesn't need to know the specific
value, or what the define is, in theory.

>> This also fixes the impossibility of using AV_TIME_BASE_Q in pre-C99/C++
>> applications, which cannot use compound literals directly in their code.
> 
> that explains the addition of a function returning the timebase,
> i agree that this part (or something equivalent) is needed
> 
> I dont understand why the defines are deprecated though, can you
> elaborate on why you change that ?

The obvious reason is that it's pretty stupid to have two ways to
access/do the same thing.

The other reason is that it is poor practice to have downstream
user code hardcoding values, since if we ever needed to change
the internal timebase, it would cause very ugly ABI issues pretty
easily if mishandled.

- Derek