[FFmpeg-devel] [RFC] libavformat/avio.h API cleanup

[Stefano Sabatini]

On date Sunday 2011-03-06 12:50:18 +0100, Stefano Sabatini encoded:
> Hi all,
> 
> I believe that having an high level overview of the mentioned API is
> useful, so people can form an opinion of the grand plan before to dig
> into the details of each single change.
> 
> Current API consists of the URLProtocol and the AVIOContext
> (previously AVByteIOContext) interfaces, which are somehow intermixed.
> 
> URLContext API allows to read/write/seek from a protocol context,
> which is accessed through a registered URLProtocol.
> 
> AVIOContext provides unbuffered I/O operations for accessing the
> resource which is accessed through a protocol.
> 
> 
> URLContext/URLProtocol API
> ==========================
> 
> URLContext
> URLPollEntry
> 
> URL_RDONLY
> URL_WRONLY
> URL_RDWR
> URL_FLAG_NONBLOCK

Maybe we should add FLAG to the other constants.
 
> url_open_protocol
> url_alloc
> url_connect
> url_open
> url_read
> url_read_complete
> url_write
> 
> AVSEEK_SIZE
> 
> url_seek
> url_close

> url_exist

This function is problematic:
http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/102825

(BTW roundup is off now, so I can't provide a direct link to the issue)

> url_filesize

missing verb, I suggest *_get_filesize()

> url_get_file_handle
> url_get_max_packet_size
> url_get_filename
> url_set_interrupt_cb
> url_poll
> av_url_read_pause
> av_url_read_seek
> 
> AVSEEK_FORCE
> 
> // URLProtocol API
> URLProtocol
> av_protocol_next
> av_register_protocol2

All these functions should be av_ prefixed, and in general I believe
using "url" in the functions is a bad idea.

Maybe something like "avproto" may fit, so we could have:
AVProtocol
AVProtocolContext

and use consistently an "avproto_" (or "avprotocol_" or
"av_proto[col]") prefix.

avproto_alloc
avproto_alloc_context
avproto_open
avproto_read
avproto_write
...
avproto_close
...
avproto_register
av_next_protocol

url_open() and url_open_protocol() may become:

int avproto_open(AVProtocolContext **h, const char *url, int flags);
int avproto_open_with_proto(AVProtocolContext **h,
                            struct AVProtocol *proto, const char *url, int flags);

Another option would be to change the API, we could have a single:
int avproto_open(AVProtocolContext **h, AVProtocol *proto, const char *url, int flags);

where proto may be NULL.

> 
> Bytestream IO context
> =====================
> 
> AVIOContext

I find the name "AVIO" a bit too generic, maybe AVByteIO was a bit
better, but imo a minor issue so the new name can stay.

> 
> avio_alloc_context
> 
> avio_write
> avio_w8
> avio_wl64
> avio_wb64
> avio_wl32
> avio_wb32
> avio_wl24
> avio_wb24
> avio_wl16
> avio_wb16

Consistent with libavutil/intreadwrite.h, lgtm.

> avio_put_str
> avio_put_str16le

Is there a reason for which we keep "put" rather than "write"?

> avio_seek
> avio_tell
> 

> url_fsize

poor name and semantically conflicting with url_filesize
(avio_get_filesize() is an option, I need to double check the
semantics though)

> url_feof
> url_ferror
> 
> av_url_read_fpause
> av_url_read_fseek
> 
> url_fgetc
> url_fprintf
> url_fgets
> 
> put_flush_packet
> 
> avio_read
> avio_r8
> avio_rl16
> avio_rl24
> avio_rl32
> avio_rl64
> avio_rb16
> avio_rb24
> avio_rb32
> avio_rb64
> 
> avio_get_str
> avio_get_str16le
> avio_get_str16be
> 

> ff_get_v

gives no hint about what the function does, also private functions
should be moved to internal headers, right?

> url_is_streamed
> url_fdopen
> url_setbufsize
> 
> ff_rewind_with_probe_data
> 
> avio_open
> avio_close
> 

> url_fileno

poor name

> url_fget_max_packet_size
> url_open_buf
> url_close_buf
> 
> url_open_dyn_buf
> url_open_dyn_packet_buf
> url_close_dyn_buf
> 

> ff_crc04C11DB7_update

I know there is a reason for this funny name, can someone refresh my
memory?

> get_checksum
> init_checksum

avio_ prefix looks reasonable at first sight.
-- 
FFmpeg = Fabulous and Foolish Merciless Patchable Elitist Game