Re: [libav-devel] [PATCH] rmdec: Move SIPR code shared with Matroska demuxer to a separate file

2012-10-15 Thread Kostya Shishkov
On Mon, Oct 15, 2012 at 01:35:49PM +0200, Diego Biurrun wrote:
> ---
> Now calling the files rmsipr.[ch].
> 
>  libavformat/Makefile  |6 ++--
>  libavformat/matroskadec.c |2 +-
>  libavformat/rm.h  |8 --
>  libavformat/rmdec.c   |   42 +--
>  libavformat/rmsipr.c  |   61 
> +
>  libavformat/rmsipr.h  |   35 +
>  6 files changed, 101 insertions(+), 53 deletions(-)
>  create mode 100644 libavformat/rmsipr.c
>  create mode 100644 libavformat/rmsipr.h

LGTM
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] rmdec: Move SIPR code shared with Matroska demuxer to a separate file

2012-10-15 Thread Kostya Shishkov
On Mon, Oct 15, 2012 at 10:51:05AM +0200, Luca Barbato wrote:
> On 10/15/2012 10:44 AM, Kostya Shishkov wrote:
> > On Mon, Oct 15, 2012 at 10:38:38AM +0200, Diego Biurrun wrote:
> >> ---
> >> Now the SIPR code is moved to a separate file instead of to the shared RM 
> >> code.
> >>
> >>  libavformat/Makefile  |6 ++--
> >>  libavformat/matroskadec.c |2 +-
> >>  libavformat/rm.h  |8 --
> >>  libavformat/rmdec.c   |   42 +--
> >>  libavformat/sipr.c|   61 
> >> +
> >>  libavformat/sipr.h|   35 +
> >>  6 files changed, 101 insertions(+), 53 deletions(-)
> >>  create mode 100644 libavformat/sipr.c
> >>  create mode 100644 libavformat/sipr.h
> > 
> > I'd move it to rm.c instead.
> > That code is not Sipro codec specific, it's specific only to Sipro-in-RM or
> > the garbage container that copies it. Sipro in AVI doesn't need that at all.
> 
> It is shared between rm and mkv thus this new file, rmmkvsipr doesn't
> sound or look as good as plain sipr.

Again, it has nothing to do with MKV directly, rmsipr should be better and not
so misleading.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] rmdec: Move SIPR code shared with Matroska demuxer to a separate file

2012-10-15 Thread Luca Barbato
On 10/15/2012 10:44 AM, Kostya Shishkov wrote:
> On Mon, Oct 15, 2012 at 10:38:38AM +0200, Diego Biurrun wrote:
>> ---
>> Now the SIPR code is moved to a separate file instead of to the shared RM 
>> code.
>>
>>  libavformat/Makefile  |6 ++--
>>  libavformat/matroskadec.c |2 +-
>>  libavformat/rm.h  |8 --
>>  libavformat/rmdec.c   |   42 +--
>>  libavformat/sipr.c|   61 
>> +
>>  libavformat/sipr.h|   35 +
>>  6 files changed, 101 insertions(+), 53 deletions(-)
>>  create mode 100644 libavformat/sipr.c
>>  create mode 100644 libavformat/sipr.h
> 
> I'd move it to rm.c instead.
> That code is not Sipro codec specific, it's specific only to Sipro-in-RM or
> the garbage container that copies it. Sipro in AVI doesn't need that at all.

It is shared between rm and mkv thus this new file, rmmkvsipr doesn't
sound or look as good as plain sipr.

lu

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] rmdec: Move SIPR code shared with Matroska demuxer to a separate file

2012-10-15 Thread Kostya Shishkov
On Mon, Oct 15, 2012 at 10:38:38AM +0200, Diego Biurrun wrote:
> ---
> Now the SIPR code is moved to a separate file instead of to the shared RM 
> code.
> 
>  libavformat/Makefile  |6 ++--
>  libavformat/matroskadec.c |2 +-
>  libavformat/rm.h  |8 --
>  libavformat/rmdec.c   |   42 +--
>  libavformat/sipr.c|   61 
> +
>  libavformat/sipr.h|   35 +
>  6 files changed, 101 insertions(+), 53 deletions(-)
>  create mode 100644 libavformat/sipr.c
>  create mode 100644 libavformat/sipr.h

I'd move it to rm.c instead.
That code is not Sipro codec specific, it's specific only to Sipro-in-RM or
the garbage container that copies it. Sipro in AVI doesn't need that at all.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel