[FFmpeg-devel] Realmedia patch

[Benoit Fouet]

Ronald S. Bultje wrote:
> Hi,
>
> On Wed, Aug 27, 2008 at 10:29 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>   
>> On Wed, Aug 27, 2008 at 10:07 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>     
>>> On Wed, Aug 27, 2008 at 09:06:44AM -0400, Ronald S. Bultje wrote:
>>>       
>>>> On Wed, Aug 27, 2008 at 8:43 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>         
>>>>> inOn Wed, Aug 27, 2008 at 08:39:11AM -0400, Ronald S. Bultje wrote:
>>>>>           
>>>>>> On Wed, Aug 27, 2008 at 8:27 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>>>             
>>>>>>> Let me try again :(
>>>>>>>
>>>>>>> ff_data_to_hex must not be in a public header not directly or indirectly.
>>>>>>> NO public header must EVER include a non public header.
>>>>>>> avformat.h is a public header
>>>>>>>               
>>>>>> Sorry, the same scheme is used in lavu/common.h, I just moved on from there...
>>>>>>             
>>>>> no
>>>>>
>>>>> lavu/common.h
>>>>> uses:
>>>>> #ifdef HAVE_AV_CONFIG_H
>>>>> #    include "internal.h"
>>>>> #endif /* HAVE_AV_CONFIG_H */
>>>>>
>>>>> So it only includes the internal header when common.h is used internally
>>>>>           
>>>> Same here, see grep HAVE_AV_CONFIG_H libavformat/avformat.h.
>>>>         
>>> hmm, true
>>>
>>> Still IMHO its clearer if internal.h is included directly instead of over
>>> a public headetr when HAVE_AV_CONFIG_H is defined.
>>>       
>> OK, fine with me. If Mans doesn't like internal.h, I'll need
>> suggestions for a name...
>>     
>
> Well, I guess if there's no better name then I'll just stick to the
> "vague" internal.h. :-). I'd request that naming-complaints are from
> now accompanied by suggestions for better names, otherwise I really
> don't know what to do with it to keep everyone happy.
>
>   

no naming request here, but...

> Index: ffmpeg-svn/libavformat/utils.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/utils.c	2008-08-26 20:28:38.000000000 -0400
> +++ ffmpeg-svn/libavformat/utils.c	2008-08-27 16:02:47.000000000 -0400
> @@ -19,6 +19,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  #include "avformat.h"
> +#include "internal.h"
>  #include "libavcodec/opt.h"
>  #include "libavutil/avstring.h"
>  #include "riff.h"
> @@ -3206,6 +3207,27 @@
>      }
>  }
>  
> +static void digit_to_char(char *dst, uint8_t src)
> +{
> +    if (src < 10) {
> +        *dst = '0' + src;
> +    } else {
> +        *dst = 'A' + src - 10;
> +    }
> +}
> +
> +char *ff_data_to_hex(char *buff, const uint8_t *src, int s)
> +{
> +    int i;
> +
> +    for(i = 0; i < s; i++) {
> +        digit_to_char(buff + 2 * i, src[i] >> 4);
> +        digit_to_char(buff + 2 * i + 1, src[i] & 0xF);
>   

could be vertically aligned

> +    }
> +
> +    return buff;
> +}
> +
>  void av_set_pts_info(AVStream *s, int pts_wrap_bits,
>                       int pts_num, int pts_den)
>  {
> Index: ffmpeg-svn/libavformat/internal.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg-svn/libavformat/internal.h	2008-08-26 20:53:16.000000000 -0400
> @@ -0,0 +1,26 @@
>
> [...]
>
> +#ifndef FFMPEG_AVFORMAT_UTILS_H
> +#define FFMPEG_AVFORMAT_UTILS_H
>   

this doesn't fit the current file name (just don't forget to update it
when/if people come with a better idea)

-- 
Benoit Fouet
Purple Labs S.A.
www.purplelabs.com