Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, Aug 16, 2011 at 12:16 PM, Alex Converse wrote: > On Tue, Aug 16, 2011 at 11:50 AM, Alex Converse > wrote: >> On Tue, Aug 16, 2011 at 11:45 AM, Anton Khirnov wrote: >>> >>> On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse >>> wrote: On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: > --- > avconv.c | 52 > ++-- > tests/codec-regression.sh | 4 +- > tests/fate.mak | 4 +- > tests/lavf-regression.sh | 20 > tests/regression-funcs.sh | 4 +- > 5 files changed, 47 insertions(+), 37 deletions(-) > Can you explain the motivation of this? >>> >>> It seems to sane to me to not reencode (if possible) unless the user >>> explicitly asked for it. Surely it's better than current insanely >>> low-quality defaults. >>> >> >> Unfortunately copy is subtly broken in a lot of situations. >> > > Situations where it generates unexpectedly broken files by default: > > 1) places where we need the mp4_adtstoasc bsf (ADTS source, global > header container) > 2) places where we need the h264_mp4toannexb bsf > 3) places where we need the chomp bsf (zero padded audio in ASF) > 4) places where we lump codecs that are normally signaled individually > under one codec_id (e.g., PRORES (which we can't decode anyway right > now) to a lesser extent AAC). 5) The container unofficially or marginally supports the codec (e.g. wma or vorbis in mp4) > > Places where this is a behavioral regression but avconv isn't > responsible for the breakage things: > 1) Broken bitstream features (all the workarounds in h263dec) > 2) Rarely used bitstream features. We can decode a lot of oddities and > borken files that many other deocders don't support. These are the > sort of features we never incude in our output (In AAC: PCE based > channel configurations, channel element reordering, CCEs, Main and LTP > profiles: none of these play on iTunes) > > Places where this does something that the user probably doesn't expect: > 1) avconv -i in.mp3 out.wav #user probably wants pcm_s16le not ACM > 2) avconv -i in.y4m out.mkv #user probably doesn't want to slap on mkv > header on uncompressed input A consequence of falling back to reëncode where copy is unsupported (not strictly required by a copy by default proposal): 1) The user gets a reëncdoe with default options (Arguably this already happens now, but there is one rule to remember when it comes to copying: There will be a reëncode, not a copy unless copy is explicitly specified. Moving forward the user will be required to know what codecs are supported by a particular container. This may change from version to version at which point the user needs '"-c:v copy" to really be sure that he is requesting a copy). ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On 8/16/11 9:53 PM, Justin Ruggles wrote: +1. Having to put -acodec pcm_s16le when decoding to wav would be annoying. You can put almost any audio codec in wav, but people will want PCM 99% of the time. The problem is that we'd end up growing a lots of quirk-modes for that and I'd like to start avconv by cleaning it up. Probably we should had it started really small and from scratch instead of using our already thinned ffmpeg.c as base. I do agree that the end program should have such user-facing shortcuts, but I'm not sure about adding them now is a priority. lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On 08/16/2011 03:16 PM, Alex Converse wrote: > On Tue, Aug 16, 2011 at 11:50 AM, Alex Converse > wrote: >> On Tue, Aug 16, 2011 at 11:45 AM, Anton Khirnov wrote: >>> >>> On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse >>> wrote: On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: > --- > avconv.c | 52 > ++-- > tests/codec-regression.sh |4 +- > tests/fate.mak|4 +- > tests/lavf-regression.sh | 20 > tests/regression-funcs.sh |4 +- > 5 files changed, 47 insertions(+), 37 deletions(-) > Can you explain the motivation of this? >>> >>> It seems to sane to me to not reencode (if possible) unless the user >>> explicitly asked for it. Surely it's better than current insanely >>> low-quality defaults. >>> I agree about the bitrate/quality defaults not being very useful, but that can be fixed by per-codec defaults and/or by somehow signalling the encoder to choose better values at codec init. >> Unfortunately copy is subtly broken in a lot of situations. >> > > Situations where it generates unexpectedly broken files by default: > > 1) places where we need the mp4_adtstoasc bsf (ADTS source, global > header container) > 2) places where we need the h264_mp4toannexb bsf > 3) places where we need the chomp bsf (zero padded audio in ASF) > 4) places where we lump codecs that are normally signaled individually > under one codec_id (e.g., PRORES (which we can't decode anyway right > now) to a lesser extent AAC). > > Places where this is a behavioral regression but avconv isn't > responsible for the breakage things: > 1) Broken bitstream features (all the workarounds in h263dec) > 2) Rarely used bitstream features. We can decode a lot of oddities and > borken files that many other deocders don't support. These are the > sort of features we never incude in our output (In AAC: PCE based > channel configurations, channel element reordering, CCEs, Main and LTP > profiles: none of these play on iTunes) > > Places where this does something that the user probably doesn't expect: > 1) avconv -i in.mp3 out.wav #user probably wants pcm_s16le not ACM +1. Having to put -acodec pcm_s16le when decoding to wav would be annoying. You can put almost any audio codec in wav, but people will want PCM 99% of the time. I agree with Alex on the IRC vs ML thing. I'm on vacation this week so I am checking email daily and have not been on IRC. -Justin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On 16 August 2011 21:16, Alex Converse wrote: > On Tue, Aug 16, 2011 at 11:50 AM, Alex Converse > wrote: >> On Tue, Aug 16, 2011 at 11:45 AM, Anton Khirnov wrote: >>> >>> On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse >>> wrote: On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: > --- > avconv.c | 52 > ++-- > tests/codec-regression.sh | 4 +- > tests/fate.mak | 4 +- > tests/lavf-regression.sh | 20 > tests/regression-funcs.sh | 4 +- > 5 files changed, 47 insertions(+), 37 deletions(-) > Can you explain the motivation of this? >>> >>> It seems to sane to me to not reencode (if possible) unless the user >>> explicitly asked for it. Surely it's better than current insanely >>> low-quality defaults. >>> >> >> Unfortunately copy is subtly broken in a lot of situations. >> > > Situations where it generates unexpectedly broken files by default: > > 1) places where we need the mp4_adtstoasc bsf (ADTS source, global > header container) > 2) places where we need the h264_mp4toannexb bsf > 3) places where we need the chomp bsf (zero padded audio in ASF) > 4) places where we lump codecs that are normally signaled individually > under one codec_id (e.g., PRORES (which we can't decode anyway right > now) to a lesser extent AAC). Can these situations be covered by some requirement of the bitstream format and automagic selection or avoidance of the usage of the bsf for relevant mappings? > Places where this is a behavioral regression but avconv isn't > responsible for the breakage things: > 1) Broken bitstream features (all the workarounds in h263dec) > 2) Rarely used bitstream features. We can decode a lot of oddities and > borken files that many other deocders don't support. These are the > sort of features we never incude in our output (In AAC: PCE based > channel configurations, channel element reordering, CCEs, Main and LTP > profiles: none of these play on iTunes) This is indeed more tricky to do automatically without explicitly identifying those properties and acting accordingly. And even then some properties might be unknown or not yet have detection implemented. Bleh. > Places where this does something that the user probably doesn't expect: > 1) avconv -i in.mp3 out.wav #user probably wants pcm_s16le not ACM > 2) avconv -i in.y4m out.mkv #user probably doesn't want to slap on mkv > header on uncompressed input I personally think using command lines like this under-specifies what avconv should do and would prefer mandatory explicit specification of the target format. I recognise that users might have other expectations and want to write a shorter command line or not have to think about what the command could do and expect it to do what they want it to do. Those two seem to suggest that perhaps for some container formats there is a general acceptance that they would normally contain uncompressed or compressed streams. Mapping such somehow would cause inconsistency, but perhaps it would improve usability for what people might expect anyway. Are people more strongly tied to expectations on file formats or to consistency of operation? Best regards, Rob ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, Aug 16, 2011 at 11:50 AM, Alex Converse wrote: > On Tue, Aug 16, 2011 at 11:45 AM, Anton Khirnov wrote: >> >> On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse >> wrote: >>> On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: >>> > --- >>> > avconv.c | 52 >>> > ++-- >>> > tests/codec-regression.sh | 4 +- >>> > tests/fate.mak | 4 +- >>> > tests/lavf-regression.sh | 20 >>> > tests/regression-funcs.sh | 4 +- >>> > 5 files changed, 47 insertions(+), 37 deletions(-) >>> > >>> >>> Can you explain the motivation of this? >> >> It seems to sane to me to not reencode (if possible) unless the user >> explicitly asked for it. Surely it's better than current insanely >> low-quality defaults. >> > > Unfortunately copy is subtly broken in a lot of situations. > Situations where it generates unexpectedly broken files by default: 1) places where we need the mp4_adtstoasc bsf (ADTS source, global header container) 2) places where we need the h264_mp4toannexb bsf 3) places where we need the chomp bsf (zero padded audio in ASF) 4) places where we lump codecs that are normally signaled individually under one codec_id (e.g., PRORES (which we can't decode anyway right now) to a lesser extent AAC). Places where this is a behavioral regression but avconv isn't responsible for the breakage things: 1) Broken bitstream features (all the workarounds in h263dec) 2) Rarely used bitstream features. We can decode a lot of oddities and borken files that many other deocders don't support. These are the sort of features we never incude in our output (In AAC: PCE based channel configurations, channel element reordering, CCEs, Main and LTP profiles: none of these play on iTunes) Places where this does something that the user probably doesn't expect: 1) avconv -i in.mp3 out.wav #user probably wants pcm_s16le not ACM 2) avconv -i in.y4m out.mkv #user probably doesn't want to slap on mkv header on uncompressed input ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, 16 Aug 2011 11:50:21 -0700, Alex Converse wrote: > On Tue, Aug 16, 2011 at 11:45 AM, Anton Khirnov wrote: > > > > On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse > > wrote: > >> On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: > >> > --- > >> > avconv.c | 52 > >> > ++-- > >> > tests/codec-regression.sh | 4 +- > >> > tests/fate.mak | 4 +- > >> > tests/lavf-regression.sh | 20 > >> > tests/regression-funcs.sh | 4 +- > >> > 5 files changed, 47 insertions(+), 37 deletions(-) > >> > > >> > >> Can you explain the motivation of this? > > > > It seems to sane to me to not reencode (if possible) unless the user > > explicitly asked for it. Surely it's better than current insanely > > low-quality defaults. > > > > Unfortunately copy is subtly broken in a lot of situations. > > > I've asked for opinions on IRC and didn't see anyone disagree yet. > > > > All development is supposed to take place on the ML. Not everyone is > on IRC at the same time. IRC is fine for collaboration but when all is > said and done everything should come through the list for everyone to > see. Sorry. Reverted for now. -- Anton Khirnov ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, Aug 16, 2011 at 11:45 AM, Anton Khirnov wrote: > > On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse > wrote: >> On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: >> > --- >> > avconv.c | 52 >> > ++-- >> > tests/codec-regression.sh | 4 +- >> > tests/fate.mak | 4 +- >> > tests/lavf-regression.sh | 20 >> > tests/regression-funcs.sh | 4 +- >> > 5 files changed, 47 insertions(+), 37 deletions(-) >> > >> >> Can you explain the motivation of this? > > It seems to sane to me to not reencode (if possible) unless the user > explicitly asked for it. Surely it's better than current insanely > low-quality defaults. > Unfortunately copy is subtly broken in a lot of situations. > I've asked for opinions on IRC and didn't see anyone disagree yet. > All development is supposed to take place on the ML. Not everyone is on IRC at the same time. IRC is fine for collaboration but when all is said and done everything should come through the list for everyone to see. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, 16 Aug 2011 11:36:23 -0700, Alex Converse wrote: > On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: > > --- > > avconv.c | 52 > > ++-- > > tests/codec-regression.sh | 4 +- > > tests/fate.mak | 4 +- > > tests/lavf-regression.sh | 20 > > tests/regression-funcs.sh | 4 +- > > 5 files changed, 47 insertions(+), 37 deletions(-) > > > > Can you explain the motivation of this? It seems to sane to me to not reencode (if possible) unless the user explicitly asked for it. Surely it's better than current insanely low-quality defaults. I've asked for opinions on IRC and didn't see anyone disagree yet. -- Anton Khirnov ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, Aug 16, 2011 at 11:36 AM, Alex Converse wrote: > On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: >> --- >> avconv.c | 52 >> ++-- >> tests/codec-regression.sh | 4 +- >> tests/fate.mak | 4 +- >> tests/lavf-regression.sh | 20 >> tests/regression-funcs.sh | 4 +- >> 5 files changed, 47 insertions(+), 37 deletions(-) >> > > Can you explain the motivation of this? > I also think this should have sat a bit longer. It's a big behavioral change. Many people didn't have adequate time to see this patch. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, Aug 16, 2011 at 1:33 AM, Anton Khirnov wrote: > --- > avconv.c | 52 ++-- > tests/codec-regression.sh | 4 +- > tests/fate.mak | 4 +- > tests/lavf-regression.sh | 20 > tests/regression-funcs.sh | 4 +- > 5 files changed, 47 insertions(+), 37 deletions(-) > Can you explain the motivation of this? ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
On Tue, Aug 16, 2011 at 10:33:59AM +0200, Anton Khirnov wrote: > --- > avconv.c | 52 ++-- > tests/codec-regression.sh |4 +- > tests/fate.mak|4 +- > tests/lavf-regression.sh | 20 > tests/regression-funcs.sh |4 +- > 5 files changed, 47 insertions(+), 37 deletions(-) looks good to me ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 5/6] avconv: use stream copy by default when possible.
---
avconv.c | 52 ++--
tests/codec-regression.sh |4 +-
tests/fate.mak|4 +-
tests/lavf-regression.sh | 20
tests/regression-funcs.sh |4 +-
5 files changed, 47 insertions(+), 37 deletions(-)
diff --git a/avconv.c b/avconv.c
index 7be6804..d52516e 100644
--- a/avconv.c
+++ b/avconv.c
@@ -2829,10 +2829,7 @@ static AVCodec *choose_codec(AVFormatContext *s,
AVStream *st, enum AVMediaType
}
if (!codec_name) {
-if (s->oformat) {
-st->codec->codec_id = av_guess_codec(s->oformat, NULL,
s->filename, NULL, type);
-return avcodec_find_encoder(st->codec->codec_id);
-}
+return NULL;
} else if (!strcmp(codec_name, "copy"))
st->stream_copy = 1;
else {
@@ -3075,7 +3072,7 @@ static void parse_forced_key_frames(char *kf,
OutputStream *ost,
}
}
-static OutputStream *new_output_stream(AVFormatContext *oc, enum AVMediaType
type)
+static OutputStream *new_output_stream(AVFormatContext *oc, enum AVMediaType
type, int source_idx)
{
OutputStream *ost;
AVStream *st = av_new_stream(oc, oc->nb_streams < nb_streamid_map ?
streamid_map[oc->nb_streams] : 0);
@@ -3102,6 +3099,18 @@ static OutputStream *new_output_stream(AVFormatContext
*oc, enum AVMediaType typ
ost->st= st;
st->codec->codec_type = type;
ost->enc = choose_codec(oc, st, type, codec_names);
+if (!ost->enc) {
+/* no codec specified, try copy if possible or fallback to format
default */
+if (source_idx >= 0 && avformat_query_codec(oc->oformat,
input_streams[source_idx].st->codec->codec_id,
+FF_COMPLIANCE_NORMAL) ==
1) {
+st->codec->codec_id =
input_streams[source_idx].st->codec->codec_id;
+st->stream_copy = 1;
+} else {
+st->codec->codec_id = av_guess_codec(oc->oformat, NULL,
oc->filename, NULL, type);
+ost->enc = avcodec_find_encoder(st->codec->codec_id);
+}
+}
+
if (ost->enc) {
ost->opts = filter_codec_opts(codec_opts, ost->enc->id, oc, st);
}
@@ -3113,13 +3122,13 @@ static OutputStream *new_output_stream(AVFormatContext
*oc, enum AVMediaType typ
return ost;
}
-static OutputStream *new_video_stream(AVFormatContext *oc)
+static OutputStream *new_video_stream(AVFormatContext *oc, int source_idx)
{
AVStream *st;
OutputStream *ost;
AVCodecContext *video_enc;
-ost = new_output_stream(oc, AVMEDIA_TYPE_VIDEO);
+ost = new_output_stream(oc, AVMEDIA_TYPE_VIDEO, source_idx);
st = ost->st;
if (!st->stream_copy) {
ost->frame_aspect_ratio = frame_aspect_ratio;
@@ -3228,13 +3237,13 @@ static OutputStream *new_video_stream(AVFormatContext
*oc)
return ost;
}
-static OutputStream *new_audio_stream(AVFormatContext *oc)
+static OutputStream *new_audio_stream(AVFormatContext *oc, int source_idx)
{
AVStream *st;
OutputStream *ost;
AVCodecContext *audio_enc;
-ost = new_output_stream(oc, AVMEDIA_TYPE_AUDIO);
+ost = new_output_stream(oc, AVMEDIA_TYPE_AUDIO, source_idx);
st = ost->st;
ost->bitstream_filters = audio_bitstream_filters;
@@ -3274,13 +3283,13 @@ static OutputStream *new_audio_stream(AVFormatContext
*oc)
return ost;
}
-static OutputStream *new_data_stream(AVFormatContext *oc)
+static OutputStream *new_data_stream(AVFormatContext *oc, int source_idx)
{
AVStream *st;
OutputStream *ost;
AVCodecContext *data_enc;
-ost = new_output_stream(oc, AVMEDIA_TYPE_DATA);
+ost = new_output_stream(oc, AVMEDIA_TYPE_DATA, source_idx);
st = ost->st;
data_enc = st->codec;
if (!st->stream_copy) {
@@ -3299,13 +3308,13 @@ static OutputStream *new_data_stream(AVFormatContext
*oc)
return ost;
}
-static OutputStream *new_subtitle_stream(AVFormatContext *oc)
+static OutputStream *new_subtitle_stream(AVFormatContext *oc, int source_idx)
{
AVStream *st;
OutputStream *ost;
AVCodecContext *subtitle_enc;
-ost = new_output_stream(oc, AVMEDIA_TYPE_SUBTITLE);
+ost = new_output_stream(oc, AVMEDIA_TYPE_SUBTITLE, source_idx);
st = ost->st;
subtitle_enc = st->codec;
@@ -3407,7 +3416,7 @@ static int read_avserver_streams(AVFormatContext *s,
const char *filename)
AVCodec *codec;
codec = avcodec_find_encoder(ic->streams[i]->codec->codec_id);
-ost = new_output_stream(s, codec->type);
+ost = new_output_stream(s, codec->type, -1);
st= ost->st;
// FIXME: a more elegant solution is needed
@@ -3474,7 +3483,7 @@ static void opt_output_file(const char *filename)
/* pick the "best" stream of each type */
#define NEW_STREAM(type, index)\
if (index >= 0) {\
-ost = new_ ## type ## _stream(oc);\
+ost = new_ ## type ## _stream(oc, index);\
