Re: [RFC] libdvbpsi API changes

On Sat, Dec 19, 2009 at 7:27 PM, Christophe Massiot <[email protected]>wrote:

> Dear all,
>
> We have been discussing the future of libdvbpsi at the VideoLAN Dev Days
> with jpsaman and gatty, and we would like to propose a number of API
> changes for libdvbpsi, and release them as version 1.0. The main goals
> of theses changes are :
>
> 1/ To avoid breaking the API and ABI every time we add support for a new
> descriptor or table.
> 2/ To move semantics out of libdvbpsi into the application and keep
> libdvbpsi a basic table parser.
> 3/ To make the API more symmetrical wrt. inputs and outputs of libdvbpsi.
>
> #############################################################
> # Goal #2 involves a major API change and a modification of #
> # applications so please keep on reading !!!                #
> #############################################################
>
>
> 1/ To avoid breaking the API and ABI at every new table, we first
> propose to make a few helper functions inline, for instance :
> dvbpsi_InitPMT()
> dvbpsi_EmptyPMT()
> dvbpsi_PMTESAdd(), dvbpsi_PMTAddDescriptor(), etc.
> (same for other tables)
>
> These functions only operate on local structures and are quite small ;
> they have no links with the rest of libdvbpsi. IMHO they are not quite
> useful since you can change the fields in the structure yourself, but
> whatever. They exist and are used in the field.
>
> For the descriptors we propose a second strategy. We define two generic
> functions, whose first parameter is a descriptor type :
> dvbpsi_handle dvbpsi_DecodeDescriptor( int i_descriptor_type,
> dvbpsi_descriptor_t * )
> dvbpsi_descriptor_t *dvbpsi_GenDescriptor( int i_descriptor_type,
> dvbpsi_handle *p_decoded, int b_duplicate )
>

TO keep it symmetric I propose to use dvbpsi_GenerateDescriptor() as we use
dvbpsi_DecodeDescriptor(). Either use abreviations everywhere or write them
out.


> And then we rewrite all the descriptor-specific functions as such :
> #define dvbpsi_DecodeISO639Dr(x) (dvbpsi_iso639_dr_t
> *)dvbpsi_DecodeDescriptor(ISO639_DR, x)
> #define dvbpsi_GenISO639Dr(x, y) dvbpsi_GenDescriptor(ISO639_DR, x, y)
>
> Note that all changes proposed in #1 are source-compatible with 0.1.6.
>
>
> 2/ The same problem arises with the Attach* and Detach* functions, but
> more difficult to solve because they have a variable number of arguments
> depending on the tables. The arguments are used to set filters on events
> that you want, like a program number or a table ID.
>
> We propose to drop this filtering support from libdvbpsi. It can
> easily be done application-side, in the callback. That way, the new API
> would just look like :
> dvbpsi_handle dvbpsi_AttachTable(int i_table_type, dvbpsi_callback
> pf_callback, void *p_cb_data)
> void dvbpsi_DetachTable(dvbpsi_handle h_dvbpsi, int i_table_type)
>
> However existing applications need to be adapted to the new behaviour.
> That is, you must have on top of your callback :
> if (p_pmt->i_program_number != i_my_program)
> {
>    dvbpsi_DeletePMT( p_pmt );
>    return;
> }
> and the equivalent for other tables.
>

I we use wrapper functions for this anyway, them we can add this generic
part of code too. This will keep existing semantics.


> Also at the same time I'd like to drop the check on the version of the
> tables. In my experience there are many cases where the table is changed
> and the version_number isn't. I know it isn't compliant, but we have to
> deal with this case. So if you want the old behaviour you'd have to add
> on top of the callback :
> if (p_pmt->i_version == i_previous_version)
> {
>    dvbpsi_DeletePMT( p_pmt );
>    return;
> }
>
> Again, this proposed change is NOT source-compatible with 0.1.6.
>
>
> 3/ All application projects duplicate the same lines of code, generally
> called "WritePSISection", to split a PSI section into transport stream
> packets. Since libdvbpsi's input is TS packets, it seems logical to be
> able to output TS packets. We therefore propose to add an extra API that
> would look like :
>
> int dvbpsi_HowManyTSBuffers(dvbpsi_section_t *)
> (merely returns (size + 183) / 184)
> int dvbpsi_GenTS( uint8_t *pp_ts_buffers[], int i_nb_buffers,
> dvbpsi_section_t *, uint16_t i_pid, uint8_t *pi_cc )
>
> This change is source-compatible with 0.1.6 because it only adds new
> functions.
>
>
> Conclusions :
> If you have a great idea, or a strong opposition to the ideas presented
> here, please share.
>
> Kind hugs to everybody,
> --
> Christophe Massiot.
> _______________________________________________
> libdvbpsi-devel mailing list
> [email protected]
> http://mailman.videolan.org/listinfo/libdvbpsi-devel
>

Kind regards,

Jean-Paul Saman

_______________________________________________
libdvbpsi-devel mailing list
[email protected]
http://mailman.videolan.org/listinfo/libdvbpsi-devel