[FFmpeg-devel] [PATCH] astenc: Add an option to set the loop flag

[Michael Niedermayer]

On Wed, Dec 12, 2012 at 07:14:38PM -0300, jamal wrote:
> On 12/12/12 5:46 PM, Carl Eugen Hoyos wrote:
> >> On 12/12/12 6:19 AM, Carl Eugen Hoyos wrote:
> >>> I wondered if values like "-1" cannot be used to indicate 
> >>> "no loop"?
> > 
> >> In which option you mean using -1? Either loopend or loopstart?
> > 
> > Yes, either one of them or both.
> > 
> >> That seems less intuitive than
> >> an option that's essentially "Enable loop"...
> > 
> > If you think so, please ignore my comment, I 
> > just wanted to share my initial thoughts.
> > 
> > Carl Eugen
> 
> Another problem is that you could give one a -1 value but an actual value to the other.
> Which would take priority in such case?
> 
> I personally prefer adding an option to enable the flag, which is much more clear for 
> the end user.
> 
> I'm attaching an updated patch that doesn't reindenting lines as you requested, and with 
> an added extra warning.
> 
> Regards.

>  libavformat/astenc.c     |   11 ++++++++++-
>  libavformat/version.h    |    2 +-
>  tests/lavf-regression.sh |    2 +-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 49973fffce4b61a2e2191dc00b29384abe2239a2  0001-astenc-Add-an-option-to-set-the-loop-flag.patch
> From 4606243d5918da1e17cd842032877f3a98ebb7d2 Mon Sep 17 00:00:00 2001
> From: James Almer <jamrial at gmail.com>
> Date: Wed, 12 Dec 2012 18:46:48 -0300
> Subject: [PATCH] astenc: Add an option to set the loop flag
> 
> Also ignore the values for loopstart and loopend if the loop flag is not enabled.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavformat/astenc.c     | 11 ++++++++++-
>  libavformat/version.h    |  2 +-
>  tests/lavf-regression.sh |  2 +-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/astenc.c b/libavformat/astenc.c
> index 6121a01..3f0fc66 100644
> --- a/libavformat/astenc.c
> +++ b/libavformat/astenc.c
> @@ -33,6 +33,7 @@ typedef struct ASTMuxContext {
>      int64_t  loopstart;
>      int64_t  loopend;
>      int      fbs;
> +    int      flags;
>  } ASTMuxContext;
>  
>  #define CHECK_LOOP(type) \
> @@ -69,6 +70,7 @@ static int ast_write_header(AVFormatContext *s)
>          return AVERROR(EINVAL);
>      }
>  
> +    if (ast->flags) {
>      if (ast->loopstart && ast->loopend && ast->loopstart >= ast->loopend) {
>          av_log(s, AV_LOG_ERROR, "loopend can't be less or equal to loopstart\n");
>          return AVERROR(EINVAL);
> @@ -77,6 +79,11 @@ static int ast_write_header(AVFormatContext *s)
>      /* Convert milliseconds to samples */
>      CHECK_LOOP(start)
>      CHECK_LOOP(end)
> +    } else {
> +        if (ast->loopstart || ast->loopend)
> +            av_log(s, AV_LOG_WARNING, "Loop flag not set. Loop values will be ignored\n");
> +        ast->loopstart = ast->loopend = 0;
> +    }
>  
>      ffio_wfourcc(pb, "STRM");
>  
> @@ -85,7 +92,7 @@ static int ast_write_header(AVFormatContext *s)
>      avio_wb16(pb, codec_tag);
>      avio_wb16(pb, 16); /* Bit depth */
>      avio_wb16(pb, enc->channels);
> -    avio_wb16(pb, 0xFFFF);
> +    avio_wb16(pb, ast->flags ? 0xFFFF : 0); /* Loop flag */
>      avio_wb32(pb, enc->sample_rate);
>  
>      ast->samples = avio_tell(pb);
> @@ -175,6 +182,8 @@ static int ast_write_trailer(AVFormatContext *s)
>  
>  #define OFFSET(obj) offsetof(ASTMuxContext, obj)
>  static const AVOption options[] = {
> +  { "astflags",  "AST Muxer flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "astflags" },
> +  { "loop",      "Enable looping",  0,             AV_OPT_TYPE_CONST, {.i64 = 1}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "astflags" },

why astflags ?
isnt a loop field alone simpler ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121222/500a90f8/attachment.asc>