[FFmpeg-devel] [PATCH v2] avformat/rtpdec_rfc4175: Fix incorrect copy_offset calculation

2019-06-24 Thread Jacob Siddall
The previous calculation code did not account for the fact that the
copy_offset for the start of the frame array is at index 0, yet the
scan line number from the rfc4175 RTP header starts at 1.
This caused 2 issues to appear:
- The first scan line was being copied into the array where the second
  scan line should be. This caused the resulting video to have a green
  line at the top of it.
- Since the packet containing the last scan line would fail the
  calculation, the packet with the RTP marker would not be processed
  which caused a log message saying "Missed previous RTP marker" to be
  outputted for each frame.

Signed-off-by: Jacob Siddall 
---
Changes in v2:
  - Don't handle packet if the line number is less than 1

Section 12 in the VSF technical recommendation TR-03 specifies that the
video scan line numbers should start at 1.
http://www.videoservicesforum.org/download/technical_recommendations/VSF_TR-03_2015-11-12.pdf

 libavformat/rtpdec_rfc4175.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c
index e9c62c1..490db87 100644
--- a/libavformat/rtpdec_rfc4175.c
+++ b/libavformat/rtpdec_rfc4175.c
@@ -205,8 +205,11 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, 
PayloadContext *data,
 if (length > payload_len)
 length = payload_len;
 
+if (line < 1)
+return AVERROR_INVALIDDATA;
+
 /* prevent ill-formed packets to write after buffer's end */
-copy_offset = (line * data->width + offset) * data->pgroup / 
data->xinc;
+copy_offset = ((line - 1) * data->width + offset) * data->pgroup / 
data->xinc;
 if (copy_offset + length > data->frame_size)
 return AVERROR_INVALIDDATA;
 
-- 
2.20.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [Patch V2] lavf/qsv_scale: add scaling modes support

2019-06-24 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Li, Zhong
> Sent: Friday, June 21, 2019 5:33 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [Patch V2] lavf/qsv_scale: add scaling modes
> support
> 
> > Subject: [Patch V2] lavf/qsv_scale: add scaling modes support
> >
> > low_power mode will use a fixed HW engine (SFC), thus can offload EU
> > usage.
> > high quality mode will take EU usage (AVS sampler).
> >
> > Performance and EU usage (Render usage) comparsion on Intel(R) Xeon(R)
> > CPU E3-1225 v5 @ 3.30GHz:
> >
> > High quality mode : ffmpeg -hwaccel qsv -c:v h264_qsv -i
> > bbb_sunflower_1080p_30fps_normal_2000frames.h264 \ -vf
> > scale_qsv=w=1280:h=736:mode=hq -f null -
> > fps=389
> > RENDER usage: 28.10 (provided by MSDK metrics_monitor)
> >
> > Low Power mode: ffmpeg -hwaccel qsv -c:v h264_qsv -i
> > ~/bbb_sunflower_1080p_30fps_normal_2000frames.h264 \ -vf
> > scale_qsv=w=1280:h=736:mode=low_power -f null -
> > fps=343
> > RENDER usage: 0.00
> >
> > Low power mode (SFC) may be disabled if not supported by
> > MSDK/Driver/HW, and replaced by AVS mode interanlly.
> >
> > Signed-off-by: Zhong Li 
> > ---
> >  libavfilter/vf_scale_qsv.c | 40
> > +++-
> >  1 file changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavfilter/vf_scale_qsv.c b/libavfilter/vf_scale_qsv.c
> > index db7715f..499534e 100644
> > --- a/libavfilter/vf_scale_qsv.c
> > +++ b/libavfilter/vf_scale_qsv.c
> > @@ -69,6 +69,8 @@ enum var_name {
> >  VARS_NB
> >  };
> >
> > +#define QSV_HAVE_SCALING_CONFIG  QSV_VERSION_ATLEAST(1, 19)
> > +
> >  typedef struct QSVScaleContext {
> >  const AVClass *class;
> >
> > @@ -88,7 +90,14 @@ typedef struct QSVScaleContext {
> >  int nb_surface_ptrs_out;
> >
> >  mfxExtOpaqueSurfaceAlloc opaque_alloc;
> > -mfxExtBuffer*ext_buffers[1];
> > +
> > +#if QSV_HAVE_SCALING_CONFIG
> > +mfxExtVPPScaling scale_conf;
> > +#endif
> > +int  mode;
> > +
> > +mfxExtBuffer *ext_buffers[1 +
> > QSV_HAVE_SCALING_CONFIG];
> > +int  num_ext_buf;
> >
> >  int shift_width, shift_height;
> >
> > @@ -285,6 +294,8 @@ static int init_out_session(AVFilterContext *ctx)
> >  mfxStatus err;
> >  int i;
> >
> > +s->num_ext_buf = 0;
> > +
> >  /* extract the properties of the "master" session given to us */
> >  err = MFXQueryIMPL(device_hwctx->session, &impl);
> >  if (err == MFX_ERR_NONE)
> > @@ -357,10 +368,7 @@ static int init_out_session(AVFilterContext *ctx)
> >  s->opaque_alloc.Header.BufferId =
> > MFX_EXTBUFF_OPAQUE_SURFACE_ALLOCATION;
> >  s->opaque_alloc.Header.BufferSz = sizeof(s->opaque_alloc);
> >
> > -s->ext_buffers[0] = (mfxExtBuffer*)&s->opaque_alloc;
> > -
> > -par.ExtParam= s->ext_buffers;
> > -par.NumExtParam = FF_ARRAY_ELEMS(s->ext_buffers);
> > +s->ext_buffers[s->num_ext_buf++] =
> > + (mfxExtBuffer*)&s->opaque_alloc;
> >
> >  par.IOPattern = MFX_IOPATTERN_IN_OPAQUE_MEMORY |
> > MFX_IOPATTERN_OUT_OPAQUE_MEMORY;
> >  } else {
> > @@ -396,6 +404,18 @@ static int init_out_session(AVFilterContext *ctx)
> >  par.IOPattern = MFX_IOPATTERN_IN_VIDEO_MEMORY |
> > MFX_IOPATTERN_OUT_VIDEO_MEMORY;
> >  }
> >
> > +#if QSV_HAVE_SCALING_CONFIG
> > +memset(&s->scale_conf, 0, sizeof(mfxExtVPPScaling));
> > +s->scale_conf.Header.BufferId = MFX_EXTBUFF_VPP_SCALING;
> > +s->scale_conf.Header.BufferSz = sizeof(mfxExtVPPScaling);
> > +s->scale_conf.ScalingMode = s->mode;
> > +s->ext_buffers[s->num_ext_buf++]  =
> > (mfxExtBuffer*)&s->scale_conf;
> > +av_log(ctx, AV_LOG_VERBOSE, "Scaling mode: %"PRIu16"\n",
> > s->mode);
> > +#endif
> > +
> > +par.ExtParam= s->ext_buffers;
> > +par.NumExtParam = s->num_ext_buf;
> > +
> >  par.AsyncDepth = 1;// TODO async
> >
> >  par.vpp.In  = in_frames_hwctx->surfaces[0].Info;
> > @@ -595,6 +615,16 @@ static const AVOption options[] = {
> >  { "h",  "Output video height", OFFSET(h_expr),
> > AV_OPT_TYPE_STRING, { .str = "ih"   }, .flags = FLAGS },
> >  { "format", "Output pixel format", OFFSET(format_str),
> > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> >
> > +#if QSV_HAVE_SCALING_CONFIG
> > +{ "mode",  "set scaling mode",OFFSET(mode),
> > AV_OPT_TYPE_INT,{ .i64 = MFX_SCALING_MODE_DEFAULT},
> > MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, FLAGS,
> "mode"},
> > +{ "low_power", "low power mode",0,
> > AV_OPT_TYPE_CONST,  { .i64 = MFX_SCALING_MODE_LOWPOWER},
> INT_MIN,
> > INT_MAX, FLAGS, "mode"},
> > +{ "hq","high quality mode", 0,
> > AV_OPT_TYPE_CONST,  { .i64 = MFX_SCALING_MODE_QUALITY},
> INT_MIN,
> > INT_MAX, FLAGS, "mode"},
> > +#else
> > +{ "mode",  "(not supported)", OFFSET(mode),
> > AV_OPT_TYPE_INT,{ .i64 = 0}, 0, INT_MAX,

Re: [FFmpeg-devel] [PATCH] movsub_bsf: Fix mov2textsub regression

2019-06-24 Thread James Almer
On 6/23/2019 10:26 PM, Philip Langdale wrote:
> On Sun, 23 Jun 2019 06:46:12 +0200
> Andreas Rheinhardt  wrote:
> 
>> The mov flavour of timed text uses the first two bytes of the packet
>> as a length field. And up until 11bef2fe said length field has been
>> read correctly in the mov2textsub bsf. But since then the next two
>> bytes are read as if they were the length field. This is fixed in
>> this commit.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/movsub_bsf.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/movsub_bsf.c b/libavcodec/movsub_bsf.c
>> index 5878607061..cd48aa7bb8 100644
>> --- a/libavcodec/movsub_bsf.c
>> +++ b/libavcodec/movsub_bsf.c
>> @@ -75,8 +75,8 @@ static int mov2textsub(AVBSFContext *ctx, AVPacket
>> *pkt) return AVERROR_INVALIDDATA;
>>  }
>>  
>> -pkt->data += 2;
>>  pkt->size  = FFMIN(pkt->size - 2, AV_RB16(pkt->data));
>> +pkt->data += 2;
>>  
>>  return 0;
>>  }
> 
> LGTM
> 
> --phil

Applied to master and backported to affected branches, thanks!

This could use a fate test, for that matter. It would have caught this
mistake instantly.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH V1] lavf/flvenc: add automatic bitstream filtering

2019-06-24 Thread myp...@gmail.com
On Sat, Jun 22, 2019 at 5:54 PM Steven Liu  wrote:
>
>
>
> > 在 2019年6月22日,17:04,Jun Zhao  写道:
> >
> > From: Jun Zhao 
> >
> > add automatic bitstream filtering when mux AAC
> >
> > Signed-off-by: Jun Zhao 
> > ---
> > libavformat/flvenc.c |   27 ---
> > 1 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
> > index e4863f1..fb1dede 100644
> > --- a/libavformat/flvenc.c
> > +++ b/libavformat/flvenc.c
> > @@ -653,11 +653,9 @@ end:
> > return ret;
> > }
> >
> > -
> > -static int flv_write_header(AVFormatContext *s)
> > +static int flv_init(struct AVFormatContext *s)
> > {
> > int i;
> > -AVIOContext *pb = s->pb;
> > FLVContext *flv = s->priv_data;
> >
> > for (i = 0; i < s->nb_streams; i++) {
> > @@ -736,6 +734,15 @@ static int flv_write_header(AVFormatContext *s)
> >
> > flv->delay = AV_NOPTS_VALUE;
> >
> > +return 0;
> > +}
> > +
> > +static int flv_write_header(AVFormatContext *s)
> > +{
> > +int i;
> > +AVIOContext *pb = s->pb;
> > +FLVContext *flv = s->priv_data;
> > +
> > avio_write(pb, "FLV", 3);
> > avio_w8(pb, 1);
> > avio_w8(pb, FLV_HEADER_FLAG_HASAUDIO * !!flv->audio_par +
> > @@ -1074,6 +1081,18 @@ static int flv_write_packet(AVFormatContext *s, 
> > AVPacket *pkt)
> > return pb->error;
> > }
> >
> > +static int flv_check_bitstream(struct AVFormatContext *s, const AVPacket 
> > *pkt)
> > +{
> > +int ret = 1;
> > +AVStream *st = s->streams[pkt->stream_index];
> > +
> > +if (st->codecpar->codec_id == AV_CODEC_ID_AAC) {
> > +if (pkt->size > 2 && (AV_RB16(pkt->data) & 0xfff0) == 0xfff0)
> > +ret = ff_stream_add_bitstream_filter(st, "aac_adtstoasc", 
> > NULL);
> > +}
> > +return ret;
> > +}
> > +
> > static const AVOption options[] = {
> > { "flvflags", "FLV muxer flags", offsetof(FLVContext, flags), 
> > AV_OPT_TYPE_FLAGS, {.i64 = 0}, INT_MIN, INT_MAX, 
> > AV_OPT_FLAG_ENCODING_PARAM, "flvflags" },
> > { "aac_seq_header_detect", "Put AAC sequence header based on stream 
> > data", 0, AV_OPT_TYPE_CONST, {.i64 = FLV_AAC_SEQ_HEADER_DETECT}, INT_MIN, 
> > INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "flvflags" },
> > @@ -1099,9 +1118,11 @@ AVOutputFormat ff_flv_muxer = {
> > .priv_data_size = sizeof(FLVContext),
> > .audio_codec= CONFIG_LIBMP3LAME ? AV_CODEC_ID_MP3 : 
> > AV_CODEC_ID_ADPCM_SWF,
> > .video_codec= AV_CODEC_ID_FLV1,
> > +.init   = flv_init,
> > .write_header   = flv_write_header,
> > .write_packet   = flv_write_packet,
> > .write_trailer  = flv_write_trailer,
> > +.check_bitstream= flv_check_bitstream,
> > .codec_tag  = (const AVCodecTag* const []) {
> >   flv_video_codec_ids, flv_audio_codec_ids, 0
> >   },
> > --
> > 1.7.1
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe”.
>
> LGTM
>
> Don’t forget add add Reported-by: Yabo Wei
>

Pushed and add the Reported-by, Tks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 1/6] libavfilter/vf_overlay.c: change the comment style for the following macro defined function

2019-06-24 Thread Lance Wang
On Thu, Jun 6, 2019 at 3:10 PM  wrote:

> From: Limin Wang 
>
> Signed-off-by: Limin Wang 
> ---
>  libavfilter/vf_overlay.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
> index 0a8f089c0d..b468cedf2e 100644
> --- a/libavfilter/vf_overlay.c
> +++ b/libavfilter/vf_overlay.c
> @@ -500,7 +500,7 @@ static av_always_inline void
> blend_plane(AVFilterContext *ctx,
>  for (; k < kmax; k++) {
>  int alpha_v, alpha_h, alpha;
>
> -// average alpha for color components, improve quality
> +/* average alpha for color components, improve quality */
>  if (hsub && vsub && j+1 < src_hp && k+1 < src_wp) {
>  alpha = (a[0] + a[src->linesize[3]] +
>   a[1] + a[src->linesize[3]+1]) >> 2;
> @@ -512,10 +512,10 @@ static av_always_inline void
> blend_plane(AVFilterContext *ctx,
>  alpha = (alpha_v + alpha_h) >> 1;
>  } else
>  alpha = a[0];
> -// if the main channel has an alpha channel, alpha has to be
> calculated
> -// to create an un-premultiplied (straight) alpha value
> +/* if the main channel has an alpha channel, alpha has to be
> calculated */
> +/* to create an un-premultiplied (straight) alpha value */
>  if (main_has_alpha && alpha != 0 && alpha != 255) {
> -// average alpha for color components, improve quality
> +/* average alpha for color components, improve quality */
>  uint8_t alpha_d;
>  if (hsub && vsub && j+1 < src_hp && k+1 < src_wp) {
>  alpha_d = (da[0] + da[dst->linesize[3]] +
> @@ -556,7 +556,7 @@ static inline void alpha_composite(const AVFrame *src,
> const AVFrame *dst,
> int x, int y,
> int jobnr, int nb_jobs)
>  {
> -uint8_t alpha;  ///< the amount of overlay to blend on to main
> +uint8_t alpha;  /* the amount of overlay to blend on to main
> */
>  uint8_t *s, *sa, *d, *da;
>  int i, imax, j, jmax;
>  int slice_start, slice_end;
> @@ -587,7 +587,7 @@ static inline void alpha_composite(const AVFrame *src,
> const AVFrame *dst,
>  *d = *s;
>  break;
>  default:
> -// apply alpha compositing: main_alpha += (1-main_alpha)
> * overlay_alpha
> +/* apply alpha compositing: main_alpha += (1-main_alpha)
> * overlay_alpha */
>  *d += FAST_DIV255((255 - *d) * *s);
>  }
>  d += 1;
> --
>

ping,  who can help to review and test the patches.




> 2.21.0
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 12/13] avformat/matroskadec: Improve read error/EOF checks II

2019-06-24 Thread James Almer
On 6/23/2019 8:42 PM, Andreas Rheinhardt wrote:
> This commit fixes a number of bugs:
> 
> 1. There was no check that no read error/EOF occured during
> ebml_read_uint, ebml_read_sint and ebml_read_float.
> 2. ebml_read_ascii and ebml_read_binary did sometimes not forward
> error codes; instead they simply returned AVERROR(EIO).
> 3. In particular, AVERROR_EOF hasn't been used and no dedicated error
> message for it existed. This has been changed.
> 
> In order to reduce code duplication, the new error code NEEDS_CHECKING
> has been introduced which makes ebml_parse check the AVIOContext's
> status for errors.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> I did not combine the two lines via FFMIN as FFMIN might call avio_skip
> twice.
> 
>  libavformat/matroskadec.c | 59 +++
>  1 file changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index b923a9edff..1db7471440 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -69,6 +69,8 @@
>  #include "qtpalette.h"
>  
>  #define EBML_UNKNOWN_LENGTH  UINT64_MAX /* EBML unknown length, in uint64_t 
> */
> +#define NEEDS_CHECKING2 /* Indicates that some error checks
> + * still need to be performed */
>  
>  typedef enum {
>  EBML_NONE,
> @@ -869,7 +871,7 @@ static int ebml_read_length(MatroskaDemuxContext 
> *matroska, AVIOContext *pb,
>  
>  /*
>   * Read the next element as an unsigned int.
> - * 0 is success, < 0 is failure.
> + * Returns NEEDS_CHECKING.
>   */
>  static int ebml_read_uint(AVIOContext *pb, int size, uint64_t *num)
>  {
> @@ -880,12 +882,12 @@ static int ebml_read_uint(AVIOContext *pb, int size, 
> uint64_t *num)
>  while (n++ < size)
>  *num = (*num << 8) | avio_r8(pb);
>  
> -return 0;
> +return NEEDS_CHECKING;
>  }
>  
>  /*
>   * Read the next element as a signed int.
> - * 0 is success, < 0 is failure.
> + * Returns NEEDS_CHECKING.
>   */
>  static int ebml_read_sint(AVIOContext *pb, int size, int64_t *num)
>  {
> @@ -901,12 +903,12 @@ static int ebml_read_sint(AVIOContext *pb, int size, 
> int64_t *num)
>  *num = ((uint64_t)*num << 8) | avio_r8(pb);
>  }
>  
> -return 0;
> +return NEEDS_CHECKING;
>  }
>  
>  /*
>   * Read the next element as a float.
> - * 0 is success, < 0 is failure.
> + * Returns NEEDS_CHECKING or < 0 on obvious failure.
>   */
>  static int ebml_read_float(AVIOContext *pb, int size, double *num)
>  {
> @@ -919,24 +921,25 @@ static int ebml_read_float(AVIOContext *pb, int size, 
> double *num)
>  else
>  return AVERROR_INVALIDDATA;
>  
> -return 0;
> +return NEEDS_CHECKING;
>  }
>  
>  /*
>   * Read the next element as an ASCII string.
> - * 0 is success, < 0 is failure.
> + * 0 is success, < 0 or NEEDS_CHECKING is failure.
>   */
>  static int ebml_read_ascii(AVIOContext *pb, int size, char **str)
>  {
>  char *res;
> +int ret;
>  
>  /* EBML strings are usually not 0-terminated, so we allocate one
>   * byte more, read the string and NULL-terminate it ourselves. */
>  if (!(res = av_malloc(size + 1)))
>  return AVERROR(ENOMEM);
> -if (avio_read(pb, (uint8_t *) res, size) != size) {
> +if ((ret = avio_read(pb, (uint8_t *) res, size)) != size) {
>  av_free(res);
> -return AVERROR(EIO);
> +return ret < 0 ? ret : NEEDS_CHECKING;
>  }
>  (res)[size] = '\0';
>  av_free(*str);
> @@ -947,7 +950,7 @@ static int ebml_read_ascii(AVIOContext *pb, int size, 
> char **str)
>  
>  /*
>   * Read the next element as binary data.
> - * 0 is success, < 0 is failure.
> + * 0 is success, < 0 or NEEDS_CHECKING is failure.
>   */
>  static int ebml_read_binary(AVIOContext *pb, int length, EbmlBin *bin)
>  {
> @@ -961,11 +964,11 @@ static int ebml_read_binary(AVIOContext *pb, int 
> length, EbmlBin *bin)
>  bin->data = bin->buf->data;
>  bin->size = length;
>  bin->pos  = avio_tell(pb);
> -if (avio_read(pb, bin->data, length) != length) {
> +if ((ret = avio_read(pb, bin->data, length)) != length) {
>  av_buffer_unref(&bin->buf);
>  bin->data = NULL;
>  bin->size = 0;
> -return AVERROR(EIO);
> +return ret < 0 ? ret : NEEDS_CHECKING;
>  }
>  
>  return 0;
> @@ -1253,14 +1256,34 @@ static int ebml_parse_elem(MatroskaDemuxContext 
> *matroska,
>  case EBML_STOP:
>  return 1;
>  default:
> -if (ffio_limit(pb, length) != length)
> +if (ffio_limit(pb, length) != length) {
> +// ffio_limit emits its own error message,
> +// so we don't have to.
>  return AVERROR(EIO);
> -return avio_skip(pb, length) < 0 ? AVERROR(EIO) : 0;
> +}
> +res = avio_skip(pb, length);
> +res = res < 0 ? res : 0;
> +}
> +if (res) {
> +if (res == NEEDS_CHECKING) {
> +   

Re: [FFmpeg-devel] [PATCH 13/13] avformat/matroskadec: Improve error/EOF checks III

2019-06-24 Thread James Almer
On 6/23/2019 8:42 PM, Andreas Rheinhardt wrote:
> Up until now, when an element was skipped, it was relied upon
> ffio_limit to make sure that there is enough data available to skip.
> ffio_limit itself relies upon the availability of the file's size. As
> this needn't be available, the check has been refined: First one byte
> less than intended is skipped, then another byte is read, followed by a
> check of the error flags.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavformat/matroskadec.c | 24 +---
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 1db7471440..caabfcf6ac 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1256,13 +1256,23 @@ static int ebml_parse_elem(MatroskaDemuxContext 
> *matroska,
>  case EBML_STOP:
>  return 1;
>  default:
> -if (ffio_limit(pb, length) != length) {
> -// ffio_limit emits its own error message,
> -// so we don't have to.
> -return AVERROR(EIO);
> -}
> -res = avio_skip(pb, length);
> -res = res < 0 ? res : 0;
> +if (length) {
> +if (ffio_limit(pb, length) != length) {
> +// ffio_limit emits its own error message,
> +// so we don't have to.
> +return AVERROR(EIO);
> +}
> +if ((res = avio_skip(pb, length - 1)) >= 0) {
> +// avio_skip might take us past EOF. We check for this
> +// by skipping only length - 1 bytes, reading a byte and
> +// checking the error flags. This is done in order to check
> +// that the element has been properly skipped even when
> +// no filesize (that ffio_limit relies on) is available.
> +avio_r8(pb);
> +res = NEEDS_CHECKING;
> +}
> +} else
> +res = 0;
>  }
>  if (res) {
>  if (res == NEEDS_CHECKING) {

Applied, thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 16/37] avformat/matroskadec: Treat SimpleBlock as EBML_BIN

2019-06-24 Thread James Almer
On 5/16/2019 7:30 PM, Andreas Rheinhardt wrote:
> Up until now, the SimpleBlock was treated specially: It basically had
> its own EBML category and it was also included in the BlockGroup EBML
> syntax (although a SimpleBlock must not exist in a BlockGroup according
> to the Matroska specifications). The latter fact also meant that
> a MatroskaBlock's buffer was always unreferenced twice.
> This has been changed: The type of a SimpleBlock is now an EBML_BIN.
> The only way in which SimpleBlocks are still different is that they
> share their associated structure with another unit (namely BlockGroup).
> This is also used to unref the block: It is always unreferenced via the
> BlockGroup syntax.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavformat/matroskadec.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 380db55340..93bf72661c 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -81,7 +81,6 @@ typedef enum {
>  EBML_BIN,
>  EBML_NEST,
>  EBML_LEVEL1,
> -EBML_PASS,
>  EBML_STOP,
>  EBML_SINT,
>  EBML_TYPE_COUNT
> @@ -696,7 +695,6 @@ static const EbmlSyntax matroska_blockadditions[] = {
>  static const EbmlSyntax matroska_blockgroup[] = {
>  { MATROSKA_ID_BLOCK,  EBML_BIN,  0, offsetof(MatroskaBlock, bin) 
> },
>  { MATROSKA_ID_BLOCKADDITIONS, EBML_NEST, 0, 0, { .n = 
> matroska_blockadditions} },
> -{ MATROSKA_ID_SIMPLEBLOCK,EBML_BIN,  0, offsetof(MatroskaBlock, bin) 
> },
>  { MATROSKA_ID_BLOCKDURATION,  EBML_UINT, 0, offsetof(MatroskaBlock, 
> duration) },
>  { MATROSKA_ID_DISCARDPADDING, EBML_SINT, 0, offsetof(MatroskaBlock, 
> discard_padding) },
>  { MATROSKA_ID_BLOCKREFERENCE, EBML_SINT, 0, offsetof(MatroskaBlock, 
> reference), { .i = INT64_MIN } },
> @@ -708,7 +706,7 @@ static const EbmlSyntax matroska_blockgroup[] = {
>  static const EbmlSyntax matroska_cluster_parsing[] = {
>  { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster, 
> timecode) },
>  { MATROSKA_ID_BLOCKGROUP,  EBML_NEST, 0, 0, { .n = 
> matroska_blockgroup } },
> -{ MATROSKA_ID_SIMPLEBLOCK, EBML_PASS, 0, 0, { .n = 
> matroska_blockgroup } },
> +{ MATROSKA_ID_SIMPLEBLOCK, EBML_BIN,  0, offsetof(MatroskaBlock, 
> bin) },
>  { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
>  { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
>  { MATROSKA_ID_INFO,EBML_NONE },
> @@ -1179,7 +1177,7 @@ static int ebml_parse_elem(MatroskaDemuxContext 
> *matroska,
>  list->nb_elem++;
>  }
>  
> -if (syntax->type != EBML_PASS && syntax->type != EBML_STOP) {
> +if (syntax->type != EBML_STOP) {
>  matroska->current_id = 0;
>  if ((res = ebml_read_length(matroska, pb, &length)) < 0)
>  return res;
> @@ -1253,8 +1251,6 @@ static int ebml_parse_elem(MatroskaDemuxContext 
> *matroska,
>  level1_elem->parsed = 1;
>  }
>  return ebml_parse_nest(matroska, syntax->def.n, data);
> -case EBML_PASS:
> -return ebml_parse_id(matroska, syntax->def.n, id, data);
>  case EBML_STOP:
>  return 1;
>  default:

Applied (Last night). Thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/matroskadec: Improve read error/EOF checks I

2019-06-24 Thread James Almer
On 6/24/2019 10:08 PM, Andreas Rheinhardt wrote:
> ebml_read_num had a number of flaws:
> 
> 1. The check for read errors/EOF was totally wrong. E.g. an EBML number
> beginning with the invalid 0x00 would be considered a read error,
> although it is just invalid data.
> 2. The check for read errors/EOF was done just once, after reading the
> first byte of the EBML number. But errors/EOF can happen inbetween, of
> course, and this wasn't checked.
> 3. There was no way to distinguish when EOF should be an error (because
> the data has to be there) for which an error message should be emitted
> and when it is not necessarily an error (namely during parsing of EBML
> IDs). Such a possibility has been added and used.
> 
> All this was fixed; furthermore, the error messages for invalid EBML
> numbers were improved and useless initializations were removed.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavformat/matroskadec.c | 74 ---
>  1 file changed, 46 insertions(+), 28 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 0e9938b65e..59388efb64 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -796,33 +796,32 @@ static int ebml_level_end(MatroskaDemuxContext 
> *matroska)
>   * Returns: number of bytes read, < 0 on error
>   */
>  static int ebml_read_num(MatroskaDemuxContext *matroska, AVIOContext *pb,
> - int max_size, uint64_t *number)
> + int max_size, uint64_t *number, int eof_forbidden)
>  {
> -int read = 1, n = 1;
> -uint64_t total = 0;
> +int read, n = 1;
> +uint64_t total;
> +int64_t pos;
>  
> -/* The first byte tells us the length in bytes - avio_r8() can normally
> - * return 0, but since that's not a valid first ebmlID byte, we can
> - * use it safely here to catch EOS. */
> -if (!(total = avio_r8(pb))) {
> -/* we might encounter EOS here */
> -if (!avio_feof(pb)) {
> -int64_t pos = avio_tell(pb);
> -av_log(matroska->ctx, AV_LOG_ERROR,
> -   "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
> -   pos, pos);
> -return pb->error ? pb->error : AVERROR(EIO);
> -}
> -return AVERROR_EOF;
> -}
> +/* The first byte tells us the length in bytes - except when it is zero. 
> */
> +total = avio_r8(pb);
> +if (pb->eof_reached)
> +goto err;
>  
>  /* get the length of the EBML number */
>  read = 8 - ff_log2_tab[total];
> -if (read > max_size) {
> -int64_t pos = avio_tell(pb) - 1;
> -av_log(matroska->ctx, AV_LOG_ERROR,
> -   "Invalid EBML number size tag 0x%02x at pos %"PRIu64" 
> (0x%"PRIx64")\n",
> -   (uint8_t) total, pos, pos);
> +
> +if (!total || read > max_size) {
> +pos = avio_tell(pb) - 1;
> +if (!total) {
> +av_log(matroska->ctx, AV_LOG_ERROR,
> +   "0x00 at pos %"PRId64" (0x%"PRIx64") invalid as first 
> byte "
> +   "of an EBML number\n", pos, pos);
> +} else {
> +av_log(matroska->ctx, AV_LOG_ERROR,
> +   "Length %d indicated by an EBML number's first byte 
> 0x%02x "
> +   "at pos %"PRId64" (0x%"PRIx64") exceeds max length %d.\n",
> +   read, (uint8_t) total, pos, pos, max_size);
> +}
>  return AVERROR_INVALIDDATA;
>  }
>  
> @@ -831,9 +830,29 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, 
> AVIOContext *pb,
>  while (n++ < read)
>  total = (total << 8) | avio_r8(pb);
>  
> +if (pb->eof_reached) {
> +eof_forbidden = 1;
> +goto err;
> +}
> +
>  *number = total;
>  
>  return read;
> +
> +err:
> +pos = avio_tell(pb);
> +if (pb->error) {
> +av_log(matroska->ctx, AV_LOG_ERROR,
> +   "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
> +   pos, pos);
> +return pb->error;
> +}
> +if (eof_forbidden) {
> +av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely "
> +   "at pos. %"PRIu64" (0x%"PRIx64")\n", pos, pos);
> +return AVERROR(EIO);
> +}
> +return AVERROR_EOF;
>  }
>  
>  /**
> @@ -844,7 +863,7 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, 
> AVIOContext *pb,
>  static int ebml_read_length(MatroskaDemuxContext *matroska, AVIOContext *pb,
>  uint64_t *number)
>  {
> -int res = ebml_read_num(matroska, pb, 8, number);
> +int res = ebml_read_num(matroska, pb, 8, number, 1);
>  if (res > 0 && *number + 1 == 1ULL << (7 * res))
>  *number = EBML_UNKNOWN_LENGTH;
>  return res;
> @@ -986,7 +1005,7 @@ static int matroska_ebmlnum_uint(MatroskaDemuxContext 
> *matroska,
>  {
>  AVIOContext pb;
>  ffio_init_context(&pb, data, size, 0, NULL, NULL, NULL, NULL);
> -return ebml_rea

Re: [FFmpeg-devel] [PATCH v10 1/2] lavf/vf_transpose: add exif orientation support

2019-06-24 Thread Jun Li
On Mon, Jun 17, 2019 at 5:31 PM Jun Li  wrote:

>
>
> On Sat, Jun 15, 2019 at 7:09 PM Jun Li  wrote:
>
>>
>>
>> On Tue, Jun 11, 2019 at 7:05 PM Jun Li  wrote:
>>
>>>
>>> On Sun, Jun 9, 2019 at 2:28 PM Jun Li  wrote:
>>>
 Add exif orientation support and expose an option.
 ---
  libavfilter/hflip.h|   2 +
  libavfilter/transpose.h|  14 
  libavfilter/vf_hflip.c |  40 ++---
  libavfilter/vf_transpose.c | 168 -
  4 files changed, 192 insertions(+), 32 deletions(-)

 diff --git a/libavfilter/hflip.h b/libavfilter/hflip.h
 index 204090dbb4..4e89bae3fc 100644
 --- a/libavfilter/hflip.h
 +++ b/libavfilter/hflip.h
 @@ -35,5 +35,7 @@ typedef struct FlipContext {

  int ff_hflip_init(FlipContext *s, int step[4], int nb_planes);
  void ff_hflip_init_x86(FlipContext *s, int step[4], int nb_planes);
 +int ff_hflip_config_props(FlipContext* s, AVFilterLink *inlink);
 +int ff_hflip_filter_slice(FlipContext *s, AVFrame *in, AVFrame *out,
 int job, int nb_jobs, int vlifp);

  #endif /* AVFILTER_HFLIP_H */
 diff --git a/libavfilter/transpose.h b/libavfilter/transpose.h
 index aa262b9487..5da08bddc0 100644
 --- a/libavfilter/transpose.h
 +++ b/libavfilter/transpose.h
 @@ -34,4 +34,18 @@ enum TransposeDir {
  TRANSPOSE_VFLIP,
  };

 +enum OrientationType {
 +ORIENTATION_AUTO_TRANSPOSE = -2,
 +ORIENTATION_AUTO_FLIP = -1,
 +ORIENTATION_NONE = 0,
 +ORIENTATION_NORMAL,
 +ORIENTATION_HFLIP,
 +ORIENTATION_ROTATE180,
 +ORIENTATION_VFLIP,
 +ORIENTATION_HFLIP_ROTATE270CW,
 +ORIENTATION_ROTATE90CW,
 +ORIENTATION_HFLIP_ROTATE90CW,
 +ORIENTATION_ROTATE270CW
 +};
 +
  #endif
 diff --git a/libavfilter/vf_hflip.c b/libavfilter/vf_hflip.c
 index b77afc77fc..d24ca5c2e7 100644
 --- a/libavfilter/vf_hflip.c
 +++ b/libavfilter/vf_hflip.c
 @@ -125,9 +125,8 @@ static void hflip_qword_c(const uint8_t *ssrc,
 uint8_t *ddst, int w)
  dst[j] = src[-j];
  }

 -static int config_props(AVFilterLink *inlink)
 +int ff_hflip_config_props(FlipContext* s, AVFilterLink *inlink)
  {
 -FlipContext *s = inlink->dst->priv;
  const AVPixFmtDescriptor *pix_desc =
 av_pix_fmt_desc_get(inlink->format);
  const int hsub = pix_desc->log2_chroma_w;
  const int vsub = pix_desc->log2_chroma_h;
 @@ -144,6 +143,12 @@ static int config_props(AVFilterLink *inlink)
  return ff_hflip_init(s, s->max_step, nb_planes);
  }

 +static int config_props(AVFilterLink *inlink)
 +{
 +FlipContext *s = inlink->dst->priv;
 +return ff_hflip_config_props(s, inlink);
 +}
 +
  int ff_hflip_init(FlipContext *s, int step[4], int nb_planes)
  {
  int i;
 @@ -170,14 +175,10 @@ typedef struct ThreadData {
  AVFrame *in, *out;
  } ThreadData;

 -static int filter_slice(AVFilterContext *ctx, void *arg, int job, int
 nb_jobs)
 +int ff_hflip_filter_slice(FlipContext *s, AVFrame *in, AVFrame *out,
 int job, int nb_jobs, int vflip)
  {
 -FlipContext *s = ctx->priv;
 -ThreadData *td = arg;
 -AVFrame *in = td->in;
 -AVFrame *out = td->out;
  uint8_t *inrow, *outrow;
 -int i, plane, step;
 +int i, plane, step, outlinesize;

  for (plane = 0; plane < 4 && in->data[plane] &&
 in->linesize[plane]; plane++) {
  const int width  = s->planewidth[plane];
 @@ -187,19 +188,36 @@ static int filter_slice(AVFilterContext *ctx,
 void *arg, int job, int nb_jobs)

  step = s->max_step[plane];

 -outrow = out->data[plane] + start * out->linesize[plane];
 -inrow  = in ->data[plane] + start * in->linesize[plane] +
 (width - 1) * step;
 +if (vflip) {
 +outrow = out->data[plane] + (height - start - 1)*
 out->linesize[plane];
 +outlinesize = -out->linesize[plane];
 +} else {
 +outrow = out->data[plane] + start * out->linesize[plane];
 +outlinesize = out->linesize[plane];
 +}
 +
 +inrow = in->data[plane] + start * in->linesize[plane] +
 (width - 1) * step;
 +
  for (i = start; i < end; i++) {
  s->flip_line[plane](inrow, outrow, width);

  inrow  += in ->linesize[plane];
 -outrow += out->linesize[plane];
 +outrow += outlinesize;
  }
  }

  return 0;
  }

 +static int filter_slice(AVFilterContext *ctx, void *arg, int job, int
 nb_jobs)
 +{
 +FlipContext *s = ctx->priv;
 +ThreadData *td = arg;
 +AVFrame *in = td->in;
 +AVFrame *out = 

Re: [FFmpeg-devel] [PATCH V1 2/3] ffmpeg_opt: Respect default disposition when select audio/video

2019-06-24 Thread myp...@gmail.com
On Fri, Jun 21, 2019 at 10:36 PM Michael Niedermayer
 wrote:
>
> On Thu, Jun 20, 2019 at 12:50:33PM +0800, Jun Zhao wrote:
> > From: Jun Zhao 
> >
> > Respect default disposition when select audio/video
> >
> > Signed-off-by: Jun Zhao 
> > ---
> >  fftools/ffmpeg_opt.c |6 --
> >  1 files changed, 4 insertions(+), 2 deletions(-)
>
> this is probably ok
>
> some testcase in fate would probably be good
>
Will add some FATE test case, tks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/matroskadec: Improve read error/EOF checks I

2019-06-24 Thread Andreas Rheinhardt
ebml_read_num had a number of flaws:

1. The check for read errors/EOF was totally wrong. E.g. an EBML number
beginning with the invalid 0x00 would be considered a read error,
although it is just invalid data.
2. The check for read errors/EOF was done just once, after reading the
first byte of the EBML number. But errors/EOF can happen inbetween, of
course, and this wasn't checked.
3. There was no way to distinguish when EOF should be an error (because
the data has to be there) for which an error message should be emitted
and when it is not necessarily an error (namely during parsing of EBML
IDs). Such a possibility has been added and used.

All this was fixed; furthermore, the error messages for invalid EBML
numbers were improved and useless initializations were removed.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/matroskadec.c | 74 ---
 1 file changed, 46 insertions(+), 28 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 0e9938b65e..59388efb64 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -796,33 +796,32 @@ static int ebml_level_end(MatroskaDemuxContext *matroska)
  * Returns: number of bytes read, < 0 on error
  */
 static int ebml_read_num(MatroskaDemuxContext *matroska, AVIOContext *pb,
- int max_size, uint64_t *number)
+ int max_size, uint64_t *number, int eof_forbidden)
 {
-int read = 1, n = 1;
-uint64_t total = 0;
+int read, n = 1;
+uint64_t total;
+int64_t pos;
 
-/* The first byte tells us the length in bytes - avio_r8() can normally
- * return 0, but since that's not a valid first ebmlID byte, we can
- * use it safely here to catch EOS. */
-if (!(total = avio_r8(pb))) {
-/* we might encounter EOS here */
-if (!avio_feof(pb)) {
-int64_t pos = avio_tell(pb);
-av_log(matroska->ctx, AV_LOG_ERROR,
-   "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
-   pos, pos);
-return pb->error ? pb->error : AVERROR(EIO);
-}
-return AVERROR_EOF;
-}
+/* The first byte tells us the length in bytes - except when it is zero. */
+total = avio_r8(pb);
+if (pb->eof_reached)
+goto err;
 
 /* get the length of the EBML number */
 read = 8 - ff_log2_tab[total];
-if (read > max_size) {
-int64_t pos = avio_tell(pb) - 1;
-av_log(matroska->ctx, AV_LOG_ERROR,
-   "Invalid EBML number size tag 0x%02x at pos %"PRIu64" 
(0x%"PRIx64")\n",
-   (uint8_t) total, pos, pos);
+
+if (!total || read > max_size) {
+pos = avio_tell(pb) - 1;
+if (!total) {
+av_log(matroska->ctx, AV_LOG_ERROR,
+   "0x00 at pos %"PRId64" (0x%"PRIx64") invalid as first byte "
+   "of an EBML number\n", pos, pos);
+} else {
+av_log(matroska->ctx, AV_LOG_ERROR,
+   "Length %d indicated by an EBML number's first byte 0x%02x "
+   "at pos %"PRId64" (0x%"PRIx64") exceeds max length %d.\n",
+   read, (uint8_t) total, pos, pos, max_size);
+}
 return AVERROR_INVALIDDATA;
 }
 
@@ -831,9 +830,29 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, 
AVIOContext *pb,
 while (n++ < read)
 total = (total << 8) | avio_r8(pb);
 
+if (pb->eof_reached) {
+eof_forbidden = 1;
+goto err;
+}
+
 *number = total;
 
 return read;
+
+err:
+pos = avio_tell(pb);
+if (pb->error) {
+av_log(matroska->ctx, AV_LOG_ERROR,
+   "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
+   pos, pos);
+return pb->error;
+}
+if (eof_forbidden) {
+av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely "
+   "at pos. %"PRIu64" (0x%"PRIx64")\n", pos, pos);
+return AVERROR(EIO);
+}
+return AVERROR_EOF;
 }
 
 /**
@@ -844,7 +863,7 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, 
AVIOContext *pb,
 static int ebml_read_length(MatroskaDemuxContext *matroska, AVIOContext *pb,
 uint64_t *number)
 {
-int res = ebml_read_num(matroska, pb, 8, number);
+int res = ebml_read_num(matroska, pb, 8, number, 1);
 if (res > 0 && *number + 1 == 1ULL << (7 * res))
 *number = EBML_UNKNOWN_LENGTH;
 return res;
@@ -986,7 +1005,7 @@ static int matroska_ebmlnum_uint(MatroskaDemuxContext 
*matroska,
 {
 AVIOContext pb;
 ffio_init_context(&pb, data, size, 0, NULL, NULL, NULL, NULL);
-return ebml_read_num(matroska, &pb, FFMIN(size, 8), num);
+return ebml_read_num(matroska, &pb, FFMIN(size, 8), num, 1);
 }
 
 /*
@@ -1033,7 +1052,7 @@ static int ebml_parse(MatroskaDemuxContext *matroska, 
EbmlSyntax *syntax,
 {
 if (!matroska->current_id) {
 uint64_t id;
-int res = ebml_read_num(matroska, 

Re: [FFmpeg-devel] [PATCH 11/13] avformat/matroskadec: Improve read error/EOF checks I

2019-06-24 Thread James Almer
On 6/23/2019 8:42 PM, Andreas Rheinhardt wrote:
> ebml_read_num had a number of flaws:
> 
> 1. The check for read errors/EOF was totally wrong. E.g. an EBML number
> beginning with the invalid 0x00 would be considered a read error,
> although it is just invalid data.
> 2. The check for read errors/EOF was done just once, after reading the
> first byte of the EBML number. But errors/EOF can happen inbetween, of
> course, and this wasn't checked.
> 3. There was no way to distinguish when EOF should be an error (because
> the data has to be there) for which an error message should be emitted
> and when it is not necessarily an error (namely during parsing of EBML
> IDs). Such a possibility has been added and used.
> 
> All this was fixed; furthermore, the error messages for invalid EBML
> numbers were improved and useless initializations were removed.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavformat/matroskadec.c | 78 +++
>  1 file changed, 47 insertions(+), 31 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index f0906675e6..b923a9edff 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -796,33 +796,30 @@ static int ebml_level_end(MatroskaDemuxContext 
> *matroska)
>   * Returns: number of bytes read, < 0 on error
>   */
>  static int ebml_read_num(MatroskaDemuxContext *matroska, AVIOContext *pb,
> - int max_size, uint64_t *number)
> + int max_size, uint64_t *number, int eof_forbidden)
>  {
> -int read = 1, n = 1;
> -uint64_t total = 0;
> +int read, n = 1;
> +uint64_t total;
> +int64_t pos;
>  
> -/* The first byte tells us the length in bytes - avio_r8() can normally
> - * return 0, but since that's not a valid first ebmlID byte, we can
> - * use it safely here to catch EOS. */
> -if (!(total = avio_r8(pb))) {
> -/* we might encounter EOS here */
> -if (!avio_feof(pb)) {
> -int64_t pos = avio_tell(pb);
> -av_log(matroska->ctx, AV_LOG_ERROR,
> -   "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
> -   pos, pos);
> -return pb->error ? pb->error : AVERROR(EIO);
> -}
> -return AVERROR_EOF;
> -}
> +/* The first byte tells us the length in bytes - except when it is zero. 
> */
> +total = avio_r8(pb);
> +if (pb->eof_reached)
> +goto err;
>  
>  /* get the length of the EBML number */
> -read = 8 - ff_log2_tab[total];
> -if (read > max_size) {
> -int64_t pos = avio_tell(pb) - 1;
> -av_log(matroska->ctx, AV_LOG_ERROR,
> -   "Invalid EBML number size tag 0x%02x at pos %"PRIu64" 
> (0x%"PRIx64")\n",
> -   (uint8_t) total, pos, pos);
> +if (!total || (read = 8 - ff_log2_tab[total]) > max_size) {

Keep the read variable assignment in its own line, otherwise it may be
unset inside this statement (i know it will not be checked as is, but
that assumption may stop being true in the future).
Alternatively just set and check both total and read in two separate
statements, same as it was before this patch.

> +pos = avio_tell(pb) - 1;
> +if (!total) {
> +av_log(matroska->ctx, AV_LOG_ERROR,
> +   "0x00 at pos %"PRId64" (0x%"PRIx64") invalid as first 
> byte "
> +   "of an EBML number\n", pos, pos);
> +} else {
> +av_log(matroska->ctx, AV_LOG_ERROR,
> +   "Length %d indicated by an EBML number's first byte 
> 0x%02x "
> +   "at pos %"PRId64" (0x%"PRIx64") exceeds max length %d.\n",
> +   read, (uint8_t) total, pos, pos, max_size);
> +}
>  return AVERROR_INVALIDDATA;
>  }
>  
> @@ -831,9 +828,29 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, 
> AVIOContext *pb,
>  while (n++ < read)
>  total = (total << 8) | avio_r8(pb);
>  
> +if (pb->eof_reached) {
> +eof_forbidden = 1;
> +goto err;
> +}
> +
>  *number = total;
>  
>  return read;
> +
> +err:
> +pos = avio_tell(pb);
> +if (pb->error) {
> +av_log(matroska->ctx, AV_LOG_ERROR,
> +   "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
> +   pos, pos);
> +return pb->error;
> +}
> +if (eof_forbidden) {
> +av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely "
> +   "at pos. %"PRIu64" (0x%"PRIx64")\n", pos, pos);
> +return AVERROR(EIO);
> +}
> +return AVERROR_EOF;
>  }
>  
>  /**
> @@ -844,7 +861,7 @@ static int ebml_read_num(MatroskaDemuxContext *matroska, 
> AVIOContext *pb,
>  static int ebml_read_length(MatroskaDemuxContext *matroska, AVIOContext *pb,
>  uint64_t *number)
>  {
> -int res = ebml_read_num(matroska, pb, 8, number);
> +int res = ebml_read_num(matroska, pb, 8, number, 

Re: [FFmpeg-devel] [PATCH 04/37] avformat/matroskadec: Don't zero unnecessarily

2019-06-24 Thread James Almer
On 5/16/2019 7:29 PM, Andreas Rheinhardt wrote:
> It is only necessary to zero the initial allocated memory used to store
> the size of laced frames if the block used Xiph lacing. Otherwise no
> unintialized data was ever used, so use av_malloc instead of av_mallocz.
> 
> Also use the correct type for the allocations.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> I checked this via the --enable-memory-poisoning compile option. It
> yielded identical output to patched version without this option as well
> as to current git head; I used Alexander Noe's AVI-Mux GUI 
> (http://www.alexander-noe.com/video/amg/) to create files using Xiph
> lacing to test this on.
> I unfortunately couldn't make Clang's MemorySanitizer or its
> AddressSanitizer run under MinGW64.
>  libavformat/matroskadec.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 3b8ddc5ecb..4dd933ef74 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2814,7 +2814,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
> *matroska, uint8_t **buf,
>  
>  if (!type) {
>  *laces= 1;
> -*lace_buf = av_mallocz(sizeof(int));
> +*lace_buf = av_malloc(sizeof(**lace_buf));
>  if (!*lace_buf)
>  return AVERROR(ENOMEM);
>  
> @@ -2826,7 +2826,7 @@ static int matroska_parse_laces(MatroskaDemuxContext 
> *matroska, uint8_t **buf,
>  *laces= *data + 1;
>  data += 1;
>  size -= 1;
> -lace_size = av_mallocz(*laces * sizeof(int));
> +lace_size = av_malloc(*laces * sizeof(*lace_size));

Changed this to av_malloc_array() while at it, and applied.
Thanks.

>  if (!lace_size)
>  return AVERROR(ENOMEM);
>  
> @@ -2836,6 +2836,8 @@ static int matroska_parse_laces(MatroskaDemuxContext 
> *matroska, uint8_t **buf,
>  uint8_t temp;
>  uint32_t total = 0;
>  for (n = 0; res == 0 && n < *laces - 1; n++) {
> +lace_size[n] = 0;
> +
>  while (1) {
>  if (size <= total) {
>  res = AVERROR_INVALIDDATA;
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] configure: print_in_columns: Replace pr with awk

2019-06-24 Thread Alexander Strasser
On 2019-05-05 11:36 +0200, Alexander Strasser wrote:
>
>
> Am 5. Mai 2019 03:53:20 MESZ schrieb "Guo, Yejun" :
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> >Of
> >> Alexander Strasser
> >> Sent: Thursday, May 02, 2019 12:08 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: [FFmpeg-devel] [PATCH 1/2] configure: print_in_columns:
> >Replace pr
> >> with awk
> >>
> >> Get rid of pr dependency and write the columns strictly
> >> alphabetical for the given rows.
> >>
> >> Before pr would attempt to write pages, thus if a page
> >> boundary was reached, the output looked confusing as one
> >> couldn't see there was a new page and the alphabetical
> >> order was disrupted when scanning down one of the columns.
> >>
> >> Fixes output for sizes with width < column width, too.
> >>
> >> Fixes part of ticket #5680
> >>
> >> Signed-off-by: Alexander Strasser 
> >> ---
> >>  configure | 18 --
> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index b122b27268..81e3776060 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -3831,8 +3831,22 @@ die_unknown(){
> >>  }
> >>
> >>  print_in_columns() {
> >> -cols=$(expr $ncols / 24)
> >> -cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
> >> +tr ' ' '\n' | sort | tr '\r\n' '  ' | awk -v col_width=24 -v
> >width="$ncols" '
> >> +{
> >> +num_cols = width > col_width ? int(width / col_width) : 1;
> >> +num_rows = int((NF + num_cols-1) / num_cols);
> >> +y = x = 1;
> >> +for (y = 1; y <= num_rows; y++) {
> >> +i = y;
> >> +for (x = 1; x <= num_cols; x++) {
> >> +if (i <= NF) {
> >> +  line = sprintf("%s%-24s", line, $i);
> >
> >not sure how to use col_width instead of the magic number 24.
>
> Good catch! Fortunately it's easy to fix:
>
>   line = sprintf("%s%-" col_width "s", line, $i);
>
> Will change it locally. Thanks.

Pushed with this change and a more elaborate commit message.


  Alexander


> >> +}
> >> +i = i + num_rows;
> >> +}
> >> +print line; line = "";
> >> +}
> >> +}' | sed 's/ *$//'
> >>  }
> >>
> >>  show_list() {
> >> --
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavd/v4l2: produce a 0 byte packet when a dequeued buffer's size is unexpected

2019-06-24 Thread Alexander Strasser
On 2019-06-05 22:04 +0200, Alexander Strasser wrote:
> From: Stephan Hilb 
>
> Behave like we do for V4L2_BUF_FLAG_ERROR, implemented in commit 28f20d2ff4 .
>
> For some devices (probably also related to the V4L driver implementation)
> it happens that when invoking the ioctl DQBUF, the returned buffer is not
> of the expected size. Here are two examples for such occurrences:
>
> [video4linux2,v4l2 @ 0x258b440] Dequeued v4l2 buffer contains 609596 
> bytes, but 614400 were expected. Flags: 0x0001.
> /dev/video1: Invalid data found when processing input
>
> [video4linux2,v4l2 @ 0x225f440] Dequeued v4l2 buffer contains 609508 
> bytes, but 614400 were expected. Flags: 0x0001.
> /dev/video1: Invalid data found when processing input
>
> For the ffmpeg CLI tool this means it will stop capturing and exit.
>
> The described behaviour was observed at least with one OmniVision USB
> web cam and with some stk1160 devices.
>
> If you search the web for the error message, you will find quite a few
> instances of this problem. Some of them experienced on other devices.
>
> Probably fixes ticket #4795
>
> Signed-off-by: Alexander Strasser 
> ---
>
> This is exactly Stephan's patch except for the commit message.
>
> @Stephan: I hope you are OK with my wording in the new message.
>
> I contacted Giorgio off-list and also put him in Bcc for this email.
>
> He previously reacted, but he probably doesn't have enough time. So if
> there are no objections I intent to commit in roughly a week if no
> more issues are found and no objections are raised.

Will push soon.

The current behavior surely causes problems for some users.
So it's better drop the short frame's data, log a warning
and have applications continue capturing instead of aborting.

It's also analog to how v4l2 indev behaves when it receives
corrupted data (V4L2_BUF_FLAG_ERROR).


  Alexander

>  libavdevice/v4l2.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index a9a0ed324d..446a243cf8 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -538,11 +538,10 @@ static int mmap_read_frame(AVFormatContext *ctx, 
> AVPacket *pkt)
>  s->frame_size = buf.bytesused;
>
>  if (s->frame_size > 0 && buf.bytesused != s->frame_size) {
> -av_log(ctx, AV_LOG_ERROR,
> +av_log(ctx, AV_LOG_WARNING,
> "Dequeued v4l2 buffer contains %d bytes, but %d were 
> expected. Flags: 0x%08X.\n",
> buf.bytesused, s->frame_size, buf.flags);
> -enqueue_buffer(s, &buf);
> -return AVERROR_INVALIDDATA;
> +buf.bytesused = 0;
>  }
>  }
>
> --
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH V5 1/2] configure: sort decoder/encoder/filter/... names in alphabet order

2019-06-24 Thread Alexander Strasser
On 2019-06-03 06:24 +, Guo, Yejun wrote:
>
>
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> > avih
> > Sent: Monday, June 03, 2019 6:29 AM
> > To: FFmpeg development discussions and patches 
> > Subject: Re: [FFmpeg-devel] [PATCH V5 1/2] configure: sort
> > decoder/encoder/filter/... names in alphabet order
> >
> > > I intent to push the awk version. I will wait at least until
> > > Thursday, so people can further test, comment, or object.
> >
> > No further comments here.
>
> I'm ok with the awk version.

Finally pushed the awk version.

Thanks again for your work and comments.


  Alexander
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] libavformat/mux: Fix audio_preload

2019-06-24 Thread Andreas Rheinhardt
Commit 31f9032b added the audio_preload feature; its goal is to
interleave audio earlier than the rest. Unfortunately, it has never ever
worked, because the check for whether a packet should be interleaved
before or after another packet was completely wrong: When audio_preload
vanishes, interleave_compare_dts returns 1 if the new packet should be
interleaved earlier than the packet it is compared with and that is what
the rest of the code expects. But the codepath used when audio_preload is
set does the opposite.

Also fixes potential undefined behaviour (namely signed integer
overflow).

Signed-off-by: Andreas Rheinhardt 
---
avformat.h contains the following note about audio_preload: "not all
formats support this and unpredictable things may happen if it is used
when not supported." Has this been added because of unpredictable
results caused by the buggy check? This option seems to work fine as
long as audio_preload is not in the range of max_interleave_duration.

 libavformat/mux.c | 22 ++
 libavformat/version.h |  2 +-
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/libavformat/mux.c b/libavformat/mux.c
index 83fe1de78f..5e1ecd8485 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1001,15 +1001,21 @@ static int interleave_compare_dts(AVFormatContext *s, 
AVPacket *next,
 AVStream *st2 = s->streams[next->stream_index];
 int comp  = av_compare_ts(next->dts, st2->time_base, pkt->dts,
   st->time_base);
-if (s->audio_preload && ((st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) 
!= (st2->codecpar->codec_type == AVMEDIA_TYPE_AUDIO))) {
-int64_t ts = av_rescale_q(pkt ->dts, st ->time_base, AV_TIME_BASE_Q) - 
s->audio_preload*(st ->codecpar->codec_type == AVMEDIA_TYPE_AUDIO);
-int64_t ts2= av_rescale_q(next->dts, st2->time_base, AV_TIME_BASE_Q) - 
s->audio_preload*(st2->codecpar->codec_type == AVMEDIA_TYPE_AUDIO);
-if (ts == ts2) {
-ts= ( pkt ->dts* st->time_base.num*AV_TIME_BASE - 
s->audio_preload*(int64_t)(st ->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)* 
st->time_base.den)*st2->time_base.den
-   -( next->dts*st2->time_base.num*AV_TIME_BASE - 
s->audio_preload*(int64_t)(st2->codecpar->codec_type == 
AVMEDIA_TYPE_AUDIO)*st2->time_base.den)* st->time_base.den;
-ts2=0;
+if (s->audio_preload) {
+int preload  = st ->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
+int preload2 = st2->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
+if (preload != preload2) {
+preload  *= s->audio_preload;
+preload2 *= s->audio_preload;
+int64_t ts = av_rescale_q(pkt ->dts, st ->time_base, 
AV_TIME_BASE_Q) - preload;
+int64_t ts2= av_rescale_q(next->dts, st2->time_base, 
AV_TIME_BASE_Q) - preload2;
+if (ts == ts2) {
+ts  = ((uint64_t)pkt ->dts*st ->time_base.num*AV_TIME_BASE - 
(uint64_t)preload *st ->time_base.den)*st2->time_base.den
+- ((uint64_t)next->dts*st2->time_base.num*AV_TIME_BASE - 
(uint64_t)preload2*st2->time_base.den)*st ->time_base.den;
+ts2 = 0;
+}
+comp = (ts2 > ts) - (ts2 < ts);
 }
-comp= (ts>ts2) - (tshttps://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add DICOM Support

2019-06-24 Thread Michael Niedermayer
On Mon, Jun 24, 2019 at 09:18:13PM +0530, Shivam wrote:
> Hi!
> 
>     The code is to add DICOM Support. The patch is only for uncompressed
> dicom files using explicit value representation. I would extend it, once i
> clarify some doubts. As dicom image files contain lots of metadata about
> the patient. So, should i display that data while demuxing or should i
> ignore and only demux the image data ?. In the current patch, i have made an
> option "-metadata", which when used will print the data on the terminal
> while demuxing.

metadata should be exported to be usable by applications.

For teh API design a one test is that it should be possible to have a
dicom file as input and a format with similar features as output and not 
loose any significant data.
Printing to the terminal cannot achieve that easily.

Either way i would suggest to support only a subset of these 4000 elements
for now but try to ensure that the deisgn can handle most of the important
elements


> 
> 
> Also, in the dicomdict.c ( which is only required for implicit dicom files,
> and for metadata), till now i have only specified ~200 data elements. but
> the dicom specification, specifies a total of ~4000 data elements (would do
> binary search if all the tags are needed ). So, is there a better way to
> specifiy that data?. (I mean if we ignore the metadata about the patient,
> then we only need to specify ~150 data elements for image data).
> 
> 
> I have uploaded some samples of dicom files with explicit value
> representation here (in case needed).
> 
> https://drive.google.com/drive/folders/1V8HUNeX3EYiPLj_dcFt8C68tAh7C7v4X?usp=sharing
> 
> 
> Please comment,

breaks fate tests

make: *** [fate-codec_desc] Error 1
make: *** [fate-source] Error 1


Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/bink: Fix integer overflow in unquantize_dct_coeffs()

2019-06-24 Thread Michael Niedermayer
On Sat, Jun 22, 2019 at 08:46:20AM +0200, Michael Niedermayer wrote:
> On Fri, Jun 21, 2019 at 09:12:36AM +0200, Reimar Döffinger wrote:
> > 
> > 
> > On 18.06.2019, at 14:55, Michael Niedermayer  wrote:
> > 
> > > Fixes: signed integer overflow: -3447 * 2883584 cannot be represented in 
> > > type 'int'
> > > Fixes: 
> > > 15265/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BINK_fuzzer-5088311799971840
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > > libavcodec/bink.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavcodec/bink.c b/libavcodec/bink.c
> > > index 8392bbeeb0..d18c0ceae4 100644
> > > --- a/libavcodec/bink.c
> > > +++ b/libavcodec/bink.c
> > > @@ -702,15 +702,15 @@ static int read_dct_coeffs(BinkContext *c, 
> > > GetBitContext *gb, int32_t block[64],
> > > return quant_idx;
> > > }
> > > 
> > > -static void unquantize_dct_coeffs(int32_t block[64], const int32_t 
> > > quant[64],
> > > +static void unquantize_dct_coeffs(int32_t block[64], const uint32_t 
> > > quant[64],
> > >   int coef_count, int coef_idx[64],
> > >   const uint8_t *scan)
> > > {
> > > int i;
> > > -block[0] = (block[0] * quant[0]) >> 11;
> > > +block[0] = (int)(block[0] * quant[0]) >> 11;
> > 
> > Huh? How do you know the multiplication result will fit in an int?
> 
> its not known
> 
> 
> > IIRC casting an out-of-range value to int is undefined behaviour, or does 
> > the tool fail to check that?
> > I might miss something, but it looks to me like just replacing one 
> > undefined behaviour with another...
> 
> Its implementation defined and our codebase depends on the normal 
> twos complement behavior of signed integers.
> 
> ISO/IEC 9899:2017   C17 ballot  N2176
> 6.3.1.3 Signed and unsigned integers
> 3 Otherwise, the new type is signed and the value cannot be represented in 
> it; either the result is
> implementation-defined or an implementation-defined signal is raised.
> 
> 
> our developer.texi:
> Implementation defined behavior for signed integers is assumed to match the
> expected behavior for two's complement. Non representable values in integer
> casts are binary truncated. Shift right of signed values uses sign extension.

I intend to apply these patches soon

thx


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2



signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH V3 1/3] tools/python: add script to convert TensorFlow model (.pb) to native model (.model)

2019-06-24 Thread Pedro Arthur
Em seg, 24 de jun de 2019 às 12:24, Guo, Yejun  escreveu:
>
> yes, good idea. Do you happen to know how to apply such repo? thanks.
>
I think you should ask Michael.

> >
> > Em qua, 19 de jun de 2019 às 21:29, Guo, Yejun 
> > escreveu:
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Guo, Yejun
> > > > Sent: Thursday, June 13, 2019 1:31 PM
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Guo, Yejun 
> > > > Subject: [PATCH V3 1/3] tools/python: add script to convert TensorFlow
> > model
> > > > (.pb) to native model (.model)
> > > >
> > > > For example, given TensorFlow model file espcn.pb,
> > > > to generate native model file espcn.model, just run:
> > > > python convert.py espcn.pb
> > > >
> > > > In current implementation, the native model file is generated for
> > > > specific dnn network with hard-code python scripts maintained out of
> > ffmpeg.
> > > > For example, srcnn network used by vf_sr is generated with
> > > >
> > https://github.com/HighVoltageRocknRoll/sr/blob/master/generate_header_a
> > > > nd_model.py#L85
> > > >
> > > > In this patch, the script is designed as a general solution which
> > > > converts general TensorFlow model .pb file into .model file. The script
> > > > now has some tricky to be compatible with current implemention, will
> > > > be refined step by step.
> > > >
> > > > The script is also added into ffmpeg source tree. It is expected there
> > > > will be many more patches and community needs the ownership of it.
> > > >
> > > > Another technical direction is to do the conversion in c/c++ code within
> > > > ffmpeg source tree. While .pb file is organized with protocol buffers,
> > > > it is not easy to do such work with tiny c/c++ code, see more discussion
> > > > at http://ffmpeg.org/pipermail/ffmpeg-devel/2019-May/244496.html. So,
> > > > choose the python script.
> > > >
> > > > Signed-off-by: Guo, Yejun 
> > > > ---
> > > >  .gitignore  |   1 +
> > > >  tools/python/convert.py |  52 +
> > > >  tools/python/convert_from_tensorflow.py | 201
> > > > 
> > > >  3 files changed, 254 insertions(+)
> > > >  create mode 100644 tools/python/convert.py
> > > >  create mode 100644 tools/python/convert_from_tensorflow.py
> > >
> > > this patch set ping for review, thanks.
> > >
> > > >
> > > > diff --git a/.gitignore b/.gitignore
> > > > index 0e57cb0..2450ee8 100644
> > > > --- a/.gitignore
> > > > +++ b/.gitignore
> > > > @@ -36,3 +36,4 @@
> > > >  /lcov/
> > > >  /src
> > > >  /mapfile
> > > > +/tools/python/__pycache__/
> > > > diff --git a/tools/python/convert.py b/tools/python/convert.py
> > > > new file mode 100644
> > > > index 000..662b429
> > > > --- /dev/null
> > > > +++ b/tools/python/convert.py
> > > > @@ -0,0 +1,52 @@
> > > > +# Copyright (c) 2019 Guo Yejun
> > > > +#
> > > > +# This file is part of FFmpeg.
> > > > +#
> > > > +# FFmpeg is free software; you can redistribute it and/or
> > > > +# modify it under the terms of the GNU Lesser General Public
> > > > +# License as published by the Free Software Foundation; either
> > > > +# version 2.1 of the License, or (at your option) any later version.
> > > > +#
> > > > +# FFmpeg is distributed in the hope that it will be useful,
> > > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > > > +# Lesser General Public License for more details.
> > > > +#
> > > > +# You should have received a copy of the GNU Lesser General Public
> > > > +# License along with FFmpeg; if not, write to the Free Software
> > > > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
> > > > 02110-1301
> > USA
> > > > +#
> > > >
> > 
> > > > ==
> > > > +
> > > > +# verified with Python 3.5.2 on Ubuntu 16.04
> > > > +import argparse
> > > > +import os
> > > > +from convert_from_tensorflow import *
> > > > +
> > > > +def get_arguments():
> > > > +parser = argparse.ArgumentParser(description='generate native
> > mode
> > > > model with weights from deep learning model')
> > > > +parser.add_argument('--outdir', type=str, default='./', 
> > > > help='where to
> > put
> > > > generated files')
> > > > +parser.add_argument('--infmt', type=str, default='tensorflow',
> > > > help='format of the deep learning model')
> > > > +parser.add_argument('infile', help='path to the deep learning model
> > with
> > > > weights')
> > > > +
> > > > +return parser.parse_args()
> > > > +
> > > > +def main():
> > > > +args = get_arguments()
> > > > +
> > > > +if not os.path.isfile(args.infile):
> > > > +print('the specified input file %s does not exist' % 
> > > > args.infile)
> > > > +exit(1)
> > > > +
> > > > +if not os.path.exists(args.outdir):
> > > > +print('create output directory %s' % args.outdir)
> > > > +  

[FFmpeg-devel] [PATCH] avformat/hlsenc: temp_file usage for master playlist and vtt playlist

2019-06-24 Thread Bodecs Bela

Dear All,

currently master playlist and subtitle playlist creation does not use
temporary files even when temp_file flag is set. Most of the use cases
it is not a problem because master playlist creation happens once on the
beginning of the whole process. But if master playlist is periodically
re-created because of master_pl_refresh_rate is set, non-atomic playlist
creation may cause problems in case of live streaming. This poblem (i.e 
non-atomic playlist
creation) may also apply for subtitle playlist (vtt) creation in live 
streaming.

This patch correct this behavior by adding missing functionality.

please review this patch.

thank you in advance,

best regards,

Bela

>From 04e70ba586646b927e1b05a9df3860a635871603 Mon Sep 17 00:00:00 2001
From: Bela Bodecs 
Date: Mon, 24 Jun 2019 17:41:49 +0200
Subject: [PATCH] avformat/hlsenc: temp_file usage for master playlist and vtt
 playlist

currently master playlist and subtitle playlist creation does not use
temporary files even when temp_file flag is set. Most of the use cases
it is not a problem because master playlist creation happens once on the
beginning of the whole process. But if master playlist is periodically
re-created because of master_pl_refresh_rate is set, non-atomic playlist
creation may cause problems in case of live streaming. This patch
correct this behavior by adding this functionality.


Signed-off-by: Bela Bodecs 
---
 doc/muxers.texi  |  6 +-
 libavformat/hlsenc.c | 30 +-
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 6c5b4bb637..d969e39fff 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -893,7 +893,11 @@ This will produce segments like this:
 @item temp_file
 Write segment data to filename.tmp and rename to filename only once the 
segment is complete. A webserver
 serving up segments can be configured to reject requests to *.tmp to prevent 
access to in-progress segments
-before they have been added to the m3u8 playlist.
+before they have been added to the m3u8 playlist. This flag also affects how 
m3u8 playlist files are created.
+If this flag is set, all playlist files will written into temporary file and 
renamed after they are complete, similarly as segments are handled.
+But playlists with @code{file} protocol and with type 
(@code{hls_playlist_type}) other than @code{vod}
+are always written into temporary file regardles of this flag. Master playlist 
files (@code{master_pl_name}), if any, with @code{file} protocol,
+are always written into temporary file regardles of this flag if 
@code{master_pl_publish_rate} value is other than zero.
 
 @end table
 
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 9f5eee5491..eaeafcbb6b 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1260,8 +1260,12 @@ static int create_master_playlist(AVFormatContext *s,
 AVDictionary *options = NULL;
 unsigned int i, j;
 int m3u8_name_size, ret, bandwidth;
-char *m3u8_rel_name, *ccgroup;
+char *m3u8_rel_name = NULL, *ccgroup;
 ClosedCaptionsStream *ccs;
+const char *proto = avio_find_protocol_name(hls->master_m3u8_url);
+int is_file_proto = proto && !strcmp(proto, "file");
+int use_temp_file = is_file_proto && ((hls->flags & HLS_TEMP_FILE) || 
hls->master_publish_rate);
+char temp_filename[1024];
 
 input_vs->m3u8_created = 1;
 if (!hls->master_m3u8_created) {
@@ -1277,12 +1281,12 @@ static int create_master_playlist(AVFormatContext *s,
 }
 
 set_http_options(s, &options, hls);
-
-ret = hlsenc_io_open(s, &hls->m3u8_out, hls->master_m3u8_url, &options);
+snprintf(temp_filename, sizeof(temp_filename), use_temp_file ? "%s.tmp" : 
"%s", hls->master_m3u8_url);
+ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options);
 av_dict_free(&options);
 if (ret < 0) {
 av_log(NULL, AV_LOG_ERROR, "Failed to open master play list file 
'%s'\n",
-hls->master_m3u8_url);
+temp_filename);
 goto fail;
 }
 
@@ -1413,7 +1417,10 @@ fail:
 if(ret >=0)
 hls->master_m3u8_created = 1;
 av_freep(&m3u8_rel_name);
-hlsenc_io_close(s, &hls->m3u8_out, hls->master_m3u8_url);
+hlsenc_io_close(s, &hls->m3u8_out, temp_filename);
+if (use_temp_file)
+ff_rename(temp_filename, hls->master_m3u8_url, s);
+
 return ret;
 }
 
@@ -1424,6 +1431,7 @@ static int hls_window(AVFormatContext *s, int last, 
VariantStream *vs)
 int target_duration = 0;
 int ret = 0;
 char temp_filename[1024];
+char temp_vtt_filename[1024];
 int64_t sequence = FFMAX(hls->start_sequence, vs->sequence - 
vs->nb_entries);
 const char *proto = avio_find_protocol_name(vs->m3u8_name);
 int is_file_proto = proto && !strcmp(proto, "file");
@@ -1505,8 +1513,9 @@ static int hls_window(AVFormatContext *s, int last, 
VariantStream *vs)
 if (last && (hls->flags & HLS_OMIT_ENDLIST)==0)
 ff_hls_write

[FFmpeg-devel] [PATCH] Add DICOM Support

2019-06-24 Thread Shivam

Hi!

    The code is to add DICOM Support. The patch is only for 
uncompressed dicom files using explicit value representation. I would 
extend it, once i clarify some doubts. As dicom image files contain 
lots of metadata about the patient. So, should i display that data while 
demuxing or should i ignore and only demux the image data ?. In the 
current patch, i have made an option "-metadata", which when used will 
print the data on the terminal while demuxing.



Also, in the dicomdict.c ( which is only required for implicit dicom 
files, and for metadata), till now i have only specified ~200 data 
elements. but the dicom specification, specifies a total of ~4000 data 
elements (would do binary search if all the tags are needed ). So, is 
there a better way to specifiy that data?. (I mean if we ignore the 
metadata about the patient, then we only need to specify ~150 data 
elements for image data).



I have uploaded some samples of dicom files with explicit value 
representation here (in case needed).


https://drive.google.com/drive/folders/1V8HUNeX3EYiPLj_dcFt8C68tAh7C7v4X?usp=sharing


Please comment,

Thank you ,

Shivam Goyal

>From d394fba8d63c3a81495b42958e9abd42757d2495 Mon Sep 17 00:00:00 2001
From: Shivam Goyal 
Date: Mon, 24 Jun 2019 21:09:14 +0530
Subject: [PATCH] lavf: Add DICOM demuxer, lavc: Add DICOM decoder

---
 libavcodec/Makefile  |   1 +
 libavcodec/allcodecs.c   |   1 +
 libavcodec/avcodec.h |   1 +
 libavcodec/codec_desc.c  |   7 +
 libavcodec/dicom.c   |  94 
 libavformat/Makefile |   1 +
 libavformat/allformats.c |   1 +
 libavformat/dicom.h  |  83 +++
 libavformat/dicomdec.c   | 308 +++
 libavformat/dicomdict.c  | 280 +++
 10 files changed, 777 insertions(+)
 create mode 100644 libavcodec/dicom.c
 create mode 100644 libavformat/dicom.h
 create mode 100644 libavformat/dicomdec.c
 create mode 100644 libavformat/dicomdict.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index edccd73037..87ae3ea048 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -263,6 +263,7 @@ OBJS-$(CONFIG_DCA_DECODER) += dcadec.o dca.o dcadata.o dcahuff.o \
 OBJS-$(CONFIG_DCA_ENCODER) += dcaenc.o dca.o dcadata.o dcahuff.o \
   dcaadpcm.o
 OBJS-$(CONFIG_DDS_DECODER) += dds.o
+OBJS-$(CONFIG_DICOM_DECODER)   += dicom.o
 OBJS-$(CONFIG_DIRAC_DECODER)   += diracdec.o dirac.o diracdsp.o diractab.o \
   dirac_arith.o dirac_dwt.o dirac_vlc.o
 OBJS-$(CONFIG_DFA_DECODER) += dfa.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index d2f9a39ce5..59d7b83625 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -83,6 +83,7 @@ extern AVCodec ff_cscd_decoder;
 extern AVCodec ff_cyuv_decoder;
 extern AVCodec ff_dds_decoder;
 extern AVCodec ff_dfa_decoder;
+extern AVCodec ff_dicom_decoder;
 extern AVCodec ff_dirac_decoder;
 extern AVCodec ff_dnxhd_encoder;
 extern AVCodec ff_dnxhd_decoder;
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 586bbbca4e..5d5e71cf10 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -410,6 +410,7 @@ enum AVCodecID {
 AV_CODEC_ID_SCREENPRESSO,
 AV_CODEC_ID_RSCC,
 AV_CODEC_ID_AVS2,
+AV_CODEC_ID_DICOM,
 
 AV_CODEC_ID_Y41P = 0x8000,
 AV_CODEC_ID_AVRP,
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 4d033c20ff..edab71f815 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1656,6 +1656,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
 .long_name = NULL_IF_CONFIG_SMALL("FITS (Flexible Image Transport System)"),
 .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
 },
+{
+.id= AV_CODEC_ID_DICOM,
+.type  = AVMEDIA_TYPE_VIDEO,
+.name  = "dicom",
+.long_name = NULL_IF_CONFIG_SMALL("DICOM (Digital Imaging and Communications in Medicine)"),
+.props = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,
+},
 {
 .id= AV_CODEC_ID_IMM4,
 .type  = AVMEDIA_TYPE_VIDEO,
diff --git a/libavcodec/dicom.c b/libavcodec/dicom.c
new file mode 100644
index 00..226f3fece3
--- /dev/null
+++ b/libavcodec/dicom.c
@@ -0,0 +1,94 @@
+/*
+ * DICOM decoder
+ * Copyright (c) 2019 Shivam Goyal
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General

Re: [FFmpeg-devel] [PATCH V3 1/3] tools/python: add script to convert TensorFlow model (.pb) to native model (.model)

2019-06-24 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Pedro Arthur
> Sent: Monday, June 24, 2019 11:13 PM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH V3 1/3] tools/python: add script to convert
> TensorFlow model (.pb) to native model (.model)
> 
> LGTM.
> 
> BTW I think we should have an ffmpeg controlled repo hosting the
> scripts to train the network and also some pretrained files to easy
> testing.

yes, good idea. Do you happen to know how to apply such repo? thanks.

> 
> Em qua, 19 de jun de 2019 às 21:29, Guo, Yejun 
> escreveu:
> >
> >
> >
> > > -Original Message-
> > > From: Guo, Yejun
> > > Sent: Thursday, June 13, 2019 1:31 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Guo, Yejun 
> > > Subject: [PATCH V3 1/3] tools/python: add script to convert TensorFlow
> model
> > > (.pb) to native model (.model)
> > >
> > > For example, given TensorFlow model file espcn.pb,
> > > to generate native model file espcn.model, just run:
> > > python convert.py espcn.pb
> > >
> > > In current implementation, the native model file is generated for
> > > specific dnn network with hard-code python scripts maintained out of
> ffmpeg.
> > > For example, srcnn network used by vf_sr is generated with
> > >
> https://github.com/HighVoltageRocknRoll/sr/blob/master/generate_header_a
> > > nd_model.py#L85
> > >
> > > In this patch, the script is designed as a general solution which
> > > converts general TensorFlow model .pb file into .model file. The script
> > > now has some tricky to be compatible with current implemention, will
> > > be refined step by step.
> > >
> > > The script is also added into ffmpeg source tree. It is expected there
> > > will be many more patches and community needs the ownership of it.
> > >
> > > Another technical direction is to do the conversion in c/c++ code within
> > > ffmpeg source tree. While .pb file is organized with protocol buffers,
> > > it is not easy to do such work with tiny c/c++ code, see more discussion
> > > at http://ffmpeg.org/pipermail/ffmpeg-devel/2019-May/244496.html. So,
> > > choose the python script.
> > >
> > > Signed-off-by: Guo, Yejun 
> > > ---
> > >  .gitignore  |   1 +
> > >  tools/python/convert.py |  52 +
> > >  tools/python/convert_from_tensorflow.py | 201
> > > 
> > >  3 files changed, 254 insertions(+)
> > >  create mode 100644 tools/python/convert.py
> > >  create mode 100644 tools/python/convert_from_tensorflow.py
> >
> > this patch set ping for review, thanks.
> >
> > >
> > > diff --git a/.gitignore b/.gitignore
> > > index 0e57cb0..2450ee8 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -36,3 +36,4 @@
> > >  /lcov/
> > >  /src
> > >  /mapfile
> > > +/tools/python/__pycache__/
> > > diff --git a/tools/python/convert.py b/tools/python/convert.py
> > > new file mode 100644
> > > index 000..662b429
> > > --- /dev/null
> > > +++ b/tools/python/convert.py
> > > @@ -0,0 +1,52 @@
> > > +# Copyright (c) 2019 Guo Yejun
> > > +#
> > > +# This file is part of FFmpeg.
> > > +#
> > > +# FFmpeg is free software; you can redistribute it and/or
> > > +# modify it under the terms of the GNU Lesser General Public
> > > +# License as published by the Free Software Foundation; either
> > > +# version 2.1 of the License, or (at your option) any later version.
> > > +#
> > > +# FFmpeg is distributed in the hope that it will be useful,
> > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> > > +# Lesser General Public License for more details.
> > > +#
> > > +# You should have received a copy of the GNU Lesser General Public
> > > +# License along with FFmpeg; if not, write to the Free Software
> > > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
> > > 02110-1301
> USA
> > > +#
> > >
> 
> > > ==
> > > +
> > > +# verified with Python 3.5.2 on Ubuntu 16.04
> > > +import argparse
> > > +import os
> > > +from convert_from_tensorflow import *
> > > +
> > > +def get_arguments():
> > > +parser = argparse.ArgumentParser(description='generate native
> mode
> > > model with weights from deep learning model')
> > > +parser.add_argument('--outdir', type=str, default='./', help='where 
> > > to
> put
> > > generated files')
> > > +parser.add_argument('--infmt', type=str, default='tensorflow',
> > > help='format of the deep learning model')
> > > +parser.add_argument('infile', help='path to the deep learning model
> with
> > > weights')
> > > +
> > > +return parser.parse_args()
> > > +
> > > +def main():
> > > +args = get_arguments()
> > > +
> > > +if not os.path.isfile(args.infile):
> > > +print('the specified input file %s does not exist' % args.infile)
> > > 

Re: [FFmpeg-devel] [PATCH V3 1/3] tools/python: add script to convert TensorFlow model (.pb) to native model (.model)

2019-06-24 Thread Pedro Arthur
LGTM.

BTW I think we should have an ffmpeg controlled repo hosting the
scripts to train the network and also some pretrained files to easy
testing.

Em qua, 19 de jun de 2019 às 21:29, Guo, Yejun  escreveu:
>
>
>
> > -Original Message-
> > From: Guo, Yejun
> > Sent: Thursday, June 13, 2019 1:31 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Guo, Yejun 
> > Subject: [PATCH V3 1/3] tools/python: add script to convert TensorFlow model
> > (.pb) to native model (.model)
> >
> > For example, given TensorFlow model file espcn.pb,
> > to generate native model file espcn.model, just run:
> > python convert.py espcn.pb
> >
> > In current implementation, the native model file is generated for
> > specific dnn network with hard-code python scripts maintained out of ffmpeg.
> > For example, srcnn network used by vf_sr is generated with
> > https://github.com/HighVoltageRocknRoll/sr/blob/master/generate_header_a
> > nd_model.py#L85
> >
> > In this patch, the script is designed as a general solution which
> > converts general TensorFlow model .pb file into .model file. The script
> > now has some tricky to be compatible with current implemention, will
> > be refined step by step.
> >
> > The script is also added into ffmpeg source tree. It is expected there
> > will be many more patches and community needs the ownership of it.
> >
> > Another technical direction is to do the conversion in c/c++ code within
> > ffmpeg source tree. While .pb file is organized with protocol buffers,
> > it is not easy to do such work with tiny c/c++ code, see more discussion
> > at http://ffmpeg.org/pipermail/ffmpeg-devel/2019-May/244496.html. So,
> > choose the python script.
> >
> > Signed-off-by: Guo, Yejun 
> > ---
> >  .gitignore  |   1 +
> >  tools/python/convert.py |  52 +
> >  tools/python/convert_from_tensorflow.py | 201
> > 
> >  3 files changed, 254 insertions(+)
> >  create mode 100644 tools/python/convert.py
> >  create mode 100644 tools/python/convert_from_tensorflow.py
>
> this patch set ping for review, thanks.
>
> >
> > diff --git a/.gitignore b/.gitignore
> > index 0e57cb0..2450ee8 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -36,3 +36,4 @@
> >  /lcov/
> >  /src
> >  /mapfile
> > +/tools/python/__pycache__/
> > diff --git a/tools/python/convert.py b/tools/python/convert.py
> > new file mode 100644
> > index 000..662b429
> > --- /dev/null
> > +++ b/tools/python/convert.py
> > @@ -0,0 +1,52 @@
> > +# Copyright (c) 2019 Guo Yejun
> > +#
> > +# This file is part of FFmpeg.
> > +#
> > +# FFmpeg is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU Lesser General Public
> > +# License as published by the Free Software Foundation; either
> > +# version 2.1 of the License, or (at your option) any later version.
> > +#
> > +# FFmpeg is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +# Lesser General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU Lesser General Public
> > +# License along with FFmpeg; if not, write to the Free Software
> > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> > USA
> > +#
> > 
> > ==
> > +
> > +# verified with Python 3.5.2 on Ubuntu 16.04
> > +import argparse
> > +import os
> > +from convert_from_tensorflow import *
> > +
> > +def get_arguments():
> > +parser = argparse.ArgumentParser(description='generate native mode
> > model with weights from deep learning model')
> > +parser.add_argument('--outdir', type=str, default='./', help='where to 
> > put
> > generated files')
> > +parser.add_argument('--infmt', type=str, default='tensorflow',
> > help='format of the deep learning model')
> > +parser.add_argument('infile', help='path to the deep learning model 
> > with
> > weights')
> > +
> > +return parser.parse_args()
> > +
> > +def main():
> > +args = get_arguments()
> > +
> > +if not os.path.isfile(args.infile):
> > +print('the specified input file %s does not exist' % args.infile)
> > +exit(1)
> > +
> > +if not os.path.exists(args.outdir):
> > +print('create output directory %s' % args.outdir)
> > +os.mkdir(args.outdir)
> > +
> > +basefile = os.path.split(args.infile)[1]
> > +basefile = os.path.splitext(basefile)[0]
> > +outfile = os.path.join(args.outdir, basefile) + '.model'
> > +
> > +if args.infmt == 'tensorflow':
> > +convert_from_tensorflow(args.infile, outfile)
> > +
> > +if __name__ == '__main__':
> > +main()
> > diff --git a/tools/python/convert_from_tensorflow.py
> > b/tools/python/convert_from_tensorflow.py
> > new file mode 100644
> > index 000..37049e5
> > --- /de

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/qdm2: Do not read out of array in fix_coding_method_array()

2019-06-24 Thread Moritz Barsnick
On Mon, Jun 24, 2019 at 01:01:02 +0200, Michael Niedermayer wrote:
> +if (sb + (j + k) / 64 > 29) {
[...]
>  if (coding_method[ch][sb + (j + k) / 64][(j + k) % 64] > 
> coding_method[ch][sb][j]) {

You could do the "sb + (j + k) / 64]" calculation only once and reuse
the result. OTOH, this code is full of magic numbers (notably 30, where
your 29 derives from) which could nicely make use of macros, but don't,
so it probably doesn't matter.

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] dash: change default MP4 extension to .m4s

2019-06-24 Thread Alfred E. Heggestad

On 24/06/2019 11:24, Jeyapal, Karthick wrote:


On 6/20/19 3:00 PM, Alfred E. Heggestad wrote:



On 20/06/2019 05:19, Jeyapal, Karthick wrote:


On 6/19/19 3:08 PM, Alfred E. Heggestad wrote:

On 19/06/2019 07:21, Jeyapal, Karthick wrote:


On 6/18/19 1:48 PM, Alfred E. Heggestad wrote:

On 18/06/2019 04:02, Steven Liu wrote:

Alfred E. Heggestad  于2019年6月17日周一 下午4:02写道:


 From 923da82598bddd1ed05750427dbc71e607d296a2 Mon Sep 17 00:00:00 2001
From: "Alfred E. Heggestad" 
Date: Mon, 17 Jun 2019 09:59:04 +0200
Subject: [PATCH] dash: change default MP4 extension to .m4s

this was changed in commit 281a21ed50849e3c8c0d03005230e9fd07c24370
---
  libavformat/dashenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 3fd7e78166..a51a1da0ca 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -166,7 +166,7 @@ static struct format_string {
  const char *str;
  } formats[] = {
  { SEGMENT_TYPE_AUTO, "auto" },
-{ SEGMENT_TYPE_MP4, "mp4" },
+{ SEGMENT_TYPE_MP4, "m4s" },
  { SEGMENT_TYPE_WEBM, "webm" },
  { 0, NULL }
  };
--
2.20.1 (Apple Git-117)

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".




LGTM



the background for this is the extension for DASH media files
used to be *.m4s and it is now *.mp4


the patch is a suggestion and should be checked by the DASH experts

what is correct according to the standard ?

the media-file is not really an .mp4 file, it cannot be
played with e.g. ffplay:

 $ ffplay chunk-stream1-1.m4s

Thanks for submitting the patch. I agree that m4s should be extension for media 
segments.
mp4 should be used only for complete files.
With respect to the patch, dashenc generates either multiple segments or a single 
file(with byte range as segments) based on "single_file" option.
The default of mp4 is correct when "single_file" is enabled. But it is wrong when 
"single_file" is disabled. The proposed patch just reverses this situation.
I would suggest the patch should handle both cases correctly.


Hi,

many thanks for your review comments.

I have updated the patch based on your comments, please see below.


this code works in my application (both single and multi files)
but the code should be reviewed by someone who has better
knowledge with the code.

Thanks for sending a revised patch promptly.
I think your patch below might adversely affect the following code
  avio_printf(out, "\t\t\tformat_name, os->codec_str, bandwidth_str, 
s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);

mimetype will become "video/m4s" for if the file extension is m4s. But I am not 
sure if it is correct or if such a mimetype exists.
I guess mimetype should remain as "video/mp4" even if the file extension is m4s.
Please let me know your views on this.



I tested with ffmpeg 4.1.3 and the mimetype is video/mp4






I made a new patch which is a bit more readable. Please see here:



 From c90254066e08a8dc46f275fbc2a1d65f26608bd4 Mon Sep 17 00:00:00 2001
From: "Alfred E. Heggestad" 
Date: Thu, 20 Jun 2019 11:27:53 +0200
Subject: [PATCH] dash: split extension for MP4 into .mp4 or .m4s

---
  libavformat/dashenc.c | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 3fd7e78166..b25afb40aa 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -89,6 +89,7 @@ typedef struct OutputStream {
  int bit_rate;
  SegmentType segment_type;  /* segment type selected for this particular 
stream */
  const char *format_name;
+const char *extension_name;
  const char *single_file_name;  /* file names selected for this particular 
stream */
  const char *init_seg_name;
  const char *media_seg_name;
@@ -217,6 +218,16 @@ static const char *get_format_str(SegmentType 
segment_type) {
  return NULL;
  }

+static const char *get_extension_str(SegmentType type, int single_file)
+{
+switch (type) {
+
+case SEGMENT_TYPE_MP4:  return single_file ? "mp4" : "m4s";
+case SEGMENT_TYPE_WEBM: return "webm";
+default: return NULL;
+}
+}
+
  static int handle_io_open_error(AVFormatContext *s, int err, char *url) {
  DASHContext *c = s->priv_data;
  char errbuf[AV_ERROR_MAX_STRING_SIZE];
@@ -254,6 +265,12 @@ static int init_segment_types(AVFormatContext *s)
  av_log(s, AV_LOG_ERROR, "Could not select DASH segment type for stream 
%d\n", i);
  return AVERROR_MUXER_NOT_FOUND;
  }
+os->extension_name = get_extension_str(segment_type, c->single_file);
+if (!os->extension_name) {
+av_log(s, AV_LOG_ERROR, "Could not get extension type for stream 
%d\n", 

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: better error log message for var_stream_map content

2019-06-24 Thread Steven Liu
Bodecs Bela  于2019年6月22日周六 下午10:01写道:
>
> Dear All,
>
> When multiple variant streams are specified by var_stream_map option,
> %v is expected either in the filename or in the last sub-directory name,
> but only in one of them. When both of them contain %v string, current
> error message only states half of the "truth".
> Additionally %v may appears several times inside the last sub-directory name
> or in filename pattern.
> This patch clarifies this in the log message and in the doc also.
>
> please review this patch.
>
> best
>
> Bela
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Applied


Thanks
Steven
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] dash: change default MP4 extension to .m4s

2019-06-24 Thread Jeyapal, Karthick

On 6/20/19 3:00 PM, Alfred E. Heggestad wrote:
>
>
> On 20/06/2019 05:19, Jeyapal, Karthick wrote:
>>
>> On 6/19/19 3:08 PM, Alfred E. Heggestad wrote:
>>> On 19/06/2019 07:21, Jeyapal, Karthick wrote:

 On 6/18/19 1:48 PM, Alfred E. Heggestad wrote:
> On 18/06/2019 04:02, Steven Liu wrote:
>> Alfred E. Heggestad  于2019年6月17日周一 下午4:02写道:
>>>
>>> From 923da82598bddd1ed05750427dbc71e607d296a2 Mon Sep 17 00:00:00 
>>> 2001
>>> From: "Alfred E. Heggestad" 
>>> Date: Mon, 17 Jun 2019 09:59:04 +0200
>>> Subject: [PATCH] dash: change default MP4 extension to .m4s
>>>
>>> this was changed in commit 281a21ed50849e3c8c0d03005230e9fd07c24370
>>> ---
>>>  libavformat/dashenc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>> index 3fd7e78166..a51a1da0ca 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -166,7 +166,7 @@ static struct format_string {
>>>  const char *str;
>>>  } formats[] = {
>>>  { SEGMENT_TYPE_AUTO, "auto" },
>>> -{ SEGMENT_TYPE_MP4, "mp4" },
>>> +{ SEGMENT_TYPE_MP4, "m4s" },
>>>  { SEGMENT_TYPE_WEBM, "webm" },
>>>  { 0, NULL }
>>>  };
>>> -- 
>>> 2.20.1 (Apple Git-117)
>>>
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". 
>>
>>
>>
>> LGTM
>>
>
> the background for this is the extension for DASH media files
> used to be *.m4s and it is now *.mp4
>
>
> the patch is a suggestion and should be checked by the DASH experts
>
> what is correct according to the standard ?
>
> the media-file is not really an .mp4 file, it cannot be
> played with e.g. ffplay:
>
> $ ffplay chunk-stream1-1.m4s 
 Thanks for submitting the patch. I agree that m4s should be extension for 
 media segments.
 mp4 should be used only for complete files.
 With respect to the patch, dashenc generates either multiple segments or a 
 single file(with byte range as segments) based on "single_file" option.
 The default of mp4 is correct when "single_file" is enabled. But it is 
 wrong when "single_file" is disabled. The proposed patch just reverses 
 this situation.
 I would suggest the patch should handle both cases correctly. 
>>>
>>> Hi,
>>>
>>> many thanks for your review comments.
>>>
>>> I have updated the patch based on your comments, please see below.
>>>
>>>
>>> this code works in my application (both single and multi files)
>>> but the code should be reviewed by someone who has better
>>> knowledge with the code. 
>> Thanks for sending a revised patch promptly.
>> I think your patch below might adversely affect the following code
>>  avio_printf(out, "\t\t\t> mimeType=\"video/%s\" codecs=\"%s\"%s width=\"%d\" height=\"%d\"",
>>  i, os->format_name, os->codec_str, bandwidth_str, 
>> s->streams[i]->codecpar->width, s->streams[i]->codecpar->height);
>>
>> mimetype will become "video/m4s" for if the file extension is m4s. But I am 
>> not sure if it is correct or if such a mimetype exists.
>> I guess mimetype should remain as "video/mp4" even if the file extension is 
>> m4s.
>> Please let me know your views on this.
>>
>
> I tested with ffmpeg 4.1.3 and the mimetype is video/mp4
>
>
>  bandwidth="1011342" width="1280" height="720">
>
>
>
> I made a new patch which is a bit more readable. Please see here:
>
>
>
> From c90254066e08a8dc46f275fbc2a1d65f26608bd4 Mon Sep 17 00:00:00 2001
> From: "Alfred E. Heggestad" 
> Date: Thu, 20 Jun 2019 11:27:53 +0200
> Subject: [PATCH] dash: split extension for MP4 into .mp4 or .m4s
>
> ---
>  libavformat/dashenc.c | 23 ---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 3fd7e78166..b25afb40aa 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -89,6 +89,7 @@ typedef struct OutputStream {
>  int bit_rate;
>  SegmentType segment_type;  /* segment type selected for this particular 
> stream */
>  const char *format_name;
> +const char *extension_name;
>  const char *single_file_name;  /* file names selected for this 
> particular stream */
>  const char *init_seg_name;
>  const char *media_seg_name;
> @@ -217,6 +218,16 @@ static const char *get_format_str(SegmentType 
> segment_type) {
>  return NULL;
>  }
>
> +static const char *get_extension_str(SegmentType type, int single_file)
> +{
> +switch (type) {
> +
> +case SEGMENT_TYPE_MP4:  return single_file ? "mp4" : "