<div dir="ltr">Hi,<br><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 17, 2013 at 6:07 AM, Jean-Baptiste Kempf <span dir="ltr"><<a href="mailto:jb@videolan.org" target="_blank">jb@videolan.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 16 Sep, Tristan Matthews wrote :<br>
<div class="im">> diff --git a/modules/codec/avcodec/fourcc.c b/modules/codec/avcodec/fourcc.c<br>
> index 3746ac8..487966b 100644<br>
> --- a/modules/codec/avcodec/fourcc.c<br>
> +++ b/modules/codec/avcodec/fourcc.c<br>
> @@ -311,6 +311,8 @@ static const struct<br>
><br>
>      { VLC_CODEC_DVAUDIO, AV_CODEC_ID_DVAUDIO, AUDIO_ES },<br>
><br>
> +    { VLC_CODEC_OPUS, AV_CODEC_ID_OPUS, AUDIO_ES },<br>
> +<br>
>      { VLC_CODEC_MACE3, AV_CODEC_ID_MACE3, AUDIO_ES },<br>
>      { VLC_CODEC_MACE6, AV_CODEC_ID_MACE6, AUDIO_ES },<br>
<br>
</div>This should be a in a separated commit.<br></blockquote><div><br></div><div>OK.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> +#ifdef ENABLE_SOUT<br>
> +<br>
> +// only ever encode 20 ms at a time<br>
<br>
</div>Why?<br></blockquote><div><br></div><div>I'll change that comment to be more informative, but basically you can only encode multiples of 2.5ms at a time with opus (other sizes are rejected), and 20ms is a sane default according to Tim. I'll try and find a more concrete answer for the comment.<br>
<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
(You should prefer C comments, tbh)<br>
<div class="im"><br>
> +        // TODO: integer pcm<br>
<br>
</div>Comment is a bit confusing<br>
<div class="im"><br>
> +    enc->fmt_in.i_codec = VLC_CODEC_FL32; // ?<br>
<br>
</div>Why "?" ?<br></blockquote><div><br></div><div>If it's ok that we only ever encode float32 then I can remove the previous two comments.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
> +#define readint(buf, base) (((buf[base+3]<<24)&0xff000000)| \<br>
> +                           ((buf[base+2]<<16)&0xff0000)| \<br>
> +                           ((buf[base+1]<<8)&0xff00)| \<br>
> +                           (buf[base]&0xff))<br>
<br>
</div>Is there any reason this is not a static inline?<br>
<div class="im"><br>
> +#define writeint(buf, base, val) do{ buf[base+3]=((val)>>24)&0xff; \<br>
> +                                     buf[base+2]=((val)>>16)&0xff; \<br>
> +                                     buf[base+1]=((val)>>8)&0xff; \<br>
> +                                     buf[base]=(val)&0xff; \<br>
> +                                 }while(0)<br>
> +#define writeshort(buf, base, val) do{ buf[base+1]=((val)>>8)&0xff; \<br>
> +                                       buf[base]=(val)&0xff; \<br>
> +                                 }while(0)<br>
<br>
</div>idem?<br></blockquote><div><br></div><div>No good reason, they are only ever called in for header writing when the encoder is open. I'll fix that.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
> +    if (comment_add(&comments, &comments_length, "ENCODER=", "opus encoder from VLC media player"))<br>
</div>Please no.<br></blockquote><div><br></div><div>Should it just be libopus then?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div><div class="h5"><br>
> diff --git a/modules/mux/ogg.c b/modules/mux/ogg.c<br>
> index 99252b0..a805996 100644<br>
> --- a/modules/mux/ogg.c<br>
> +++ b/modules/mux/ogg.c<br>
> @@ -361,6 +361,10 @@ static int AddStream( sout_mux_t *p_mux, sout_input_t *p_input )<br>
>      case AUDIO_ES:<br>
>          switch( p_stream->i_fourcc )<br>
>          {<br>
> +        case VLC_CODEC_OPUS:<br>
> +            msg_Dbg( p_mux, "opus stream" );<br>
> +            break;<br>
> +<br>
>          case VLC_CODEC_VORBIS:<br>
>              msg_Dbg( p_mux, "vorbis stream" );<br>
>              break;<br>
> @@ -626,6 +630,7 @@ static block_t *OggCreateHeader( sout_mux_t *p_mux )<br>
><br>
>              if( p_stream->i_fourcc == VLC_CODEC_VORBIS ||<br>
>                  p_stream->i_fourcc == VLC_CODEC_SPEEX ||<br>
> +                p_stream->i_fourcc == VLC_CODEC_OPUS ||<br>
>                  p_stream->i_fourcc == VLC_CODEC_THEORA )<br>
>              {<br>
>                  /* First packet in order: vorbis/speex/theora info */<br>
> @@ -713,6 +718,7 @@ static block_t *OggCreateHeader( sout_mux_t *p_mux )<br>
><br>
>          if( p_stream->i_fourcc == VLC_CODEC_VORBIS ||<br>
>              p_stream->i_fourcc == VLC_CODEC_SPEEX ||<br>
> +            p_stream->i_fourcc == VLC_CODEC_OPUS ||<br>
>              p_stream->i_fourcc == VLC_CODEC_THEORA )<br>
>          {<br>
>              unsigned pi_size[XIPH_MAX_HEADER_COUNT];<br>
> @@ -977,6 +983,7 @@ static int MuxBlock( sout_mux_t *p_mux, sout_input_t *p_input )<br>
>      if( p_stream->i_fourcc != VLC_CODEC_VORBIS &&<br>
>          p_stream->i_fourcc != VLC_CODEC_FLAC &&<br>
>          p_stream->i_fourcc != VLC_CODEC_SPEEX &&<br>
> +        p_stream->i_fourcc != VLC_CODEC_OPUS &&<br>
>          p_stream->i_fourcc != VLC_CODEC_THEORA &&<br>
>          p_stream->i_fourcc != VLC_CODEC_DIRAC )<br>
>      {<br>
> @@ -994,6 +1001,7 @@ static int MuxBlock( sout_mux_t *p_mux, sout_input_t *p_input )<br>
>      {<br>
>          if( p_stream->i_fourcc == VLC_CODEC_VORBIS ||<br>
>              p_stream->i_fourcc == VLC_CODEC_FLAC ||<br>
> +            p_stream->i_fourcc == VLC_CODEC_OPUS ||<br>
>              p_stream->i_fourcc == VLC_CODEC_SPEEX )<br>
>          {<br>
>              /* number of sample from begining + current packet */<br>
<br>
</div></div></blockquote><div><br></div><div>Thanks for the feedback.<br><br></div><div>Best,<br>Tristan<br></div></div><br clear="all"><br>-- <br>Tristan Matthews<br>web: <a href="http://tristanswork.blogspot.com">http://tristanswork.blogspot.com</a><br>

</div></div></div></div>