Re: [FFmpeg-devel] [PATCH 0/6] Toward a SPS filter for H.264

2014-11-30 Thread Michael Niedermayer
On Sun, Nov 30, 2014 at 01:15:43PM +0100, Vittorio Giovara wrote:
> 
> On 30/11/2014 01:45, Christophe Gisquet wrote:
> >Most of the code is actually not mine but originated from "Direct264":
> >http://forum.doom9.org/showthread.php?t=152419
> >https://svn.code.sf.net/p/direct264/code/Patches/
> >Therefore I've tried to split as best as possible the code I have added.
> >There are 3 controversial parts in this patch set:
> >- the BSF API change (fixing it is out of my league/time budget)
> >- the filter doing in-place filtering (?)
> >- the level of validation for this new filter
> >
> >I've only tested the cropping feature, and validating that the rest
> >works tenuous. Anyway, cropping is unfortunately not at all well
> >supported:
> >- VLC 2.1.5 (Windows) either crashes or outputs garbage
> >- DXVA on some Intel core gets top/left cropping completely wrong
> >   (0,0 but something valid for bottom/right seems to be OK)
> >- ffmpeg-based decoders (e.g. mplayer) are OK
> >
> >It is possible the implementation is incorrect but it was written
> >following the specs.
> >
> I don't really understand this patchset nor the usecase for it.

I belive its usecase is to correct incorrect parameters in an SPS
croping, aspect ratio level/profile, ...
in cases where re-encoding with better parameters is not possible
(broken encoder, source material unavailble, ...


> While of course it's technically possible to change the SPS, it's a
> VeryBadIdea to so. Especially if you are targetting cropping (which
> is fixed in VLC 2.2 btw), you *never ever* have to ignore it, but
> use it to crop the picture correctly, as it's a mandatory part in
> the H264 specifications. If your use case is non-spec compliant,
> then you already have a CODEC_FLAG for that, which does not modifies
> the bitstream.
> All other uses cases seem workaround for broken encoders/decoders,
> it'd be better to fix those instead.
> 
> Vittorio
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


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


Re: [FFmpeg-devel] [PATCH 0/6] Toward a SPS filter for H.264

2014-11-30 Thread Vittorio Giovara


On 30/11/2014 01:45, Christophe Gisquet wrote:

Most of the code is actually not mine but originated from "Direct264":
http://forum.doom9.org/showthread.php?t=152419
https://svn.code.sf.net/p/direct264/code/Patches/
Therefore I've tried to split as best as possible the code I have added.
There are 3 controversial parts in this patch set:
- the BSF API change (fixing it is out of my league/time budget)
- the filter doing in-place filtering (?)
- the level of validation for this new filter

I've only tested the cropping feature, and validating that the rest
works tenuous. Anyway, cropping is unfortunately not at all well
supported:
- VLC 2.1.5 (Windows) either crashes or outputs garbage
- DXVA on some Intel core gets top/left cropping completely wrong
   (0,0 but something valid for bottom/right seems to be OK)
- ffmpeg-based decoders (e.g. mplayer) are OK

It is possible the implementation is incorrect but it was written
following the specs.


I don't really understand this patchset nor the usecase for it.
While of course it's technically possible to change the SPS, it's a 
VeryBadIdea to so. Especially if you are targetting cropping (which is 
fixed in VLC 2.2 btw), you *never ever* have to ignore it, but use it to 
crop the picture correctly, as it's a mandatory part in the H264 
specifications. If your use case is non-spec compliant, then you already 
have a CODEC_FLAG for that, which does not modifies the bitstream.
All other uses cases seem workaround for broken encoders/decoders, it'd 
be better to fix those instead.


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


[FFmpeg-devel] [PATCH 0/6] Toward a SPS filter for H.264

2014-11-29 Thread Christophe Gisquet
Most of the code is actually not mine but originated from "Direct264":
http://forum.doom9.org/showthread.php?t=152419
https://svn.code.sf.net/p/direct264/code/Patches/
Therefore I've tried to split as best as possible the code I have added.

There are 3 controversial parts in this patch set:
- the BSF API change (fixing it is out of my league/time budget)
- the filter doing in-place filtering (?)
- the level of validation for this new filter

I've only tested the cropping feature, and validating that the rest
works tenuous. Anyway, cropping is unfortunately not at all well
supported:
- VLC 2.1.5 (Windows) either crashes or outputs garbage
- DXVA on some Intel core gets top/left cropping completely wrong
  (0,0 but something valid for bottom/right seems to be OK)
- ffmpeg-based decoders (e.g. mplayer) are OK

It is possible the implementation is incorrect but it was written
following the specs.

Christophe Gisquet (4):
  h264_ps: move and export aspect_ratio
  h264_changesps_bsf: fix compilation
  h264_changesps_bsf: add and use init
  h264_changesps_bsf: allow specifying full cropping

Zongyi Zhou (2):
  bitstream_filter: add an init function
  h264_changesps_bsf: import code

 ffmpeg_opt.c|   5 +-
 libavcodec/Makefile |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/avcodec.h|   5 +-
 libavcodec/bitstream_filter.c   |   8 +-
 libavcodec/h264_changesps_bsf.c | 641 
 libavcodec/h264_ps.c|  26 +-
 libavcodec/h264data.h   |  19 ++
 libavformat/concatdec.c |   2 +-
 libavformat/tee.c   |   7 +-
 10 files changed, 687 insertions(+), 28 deletions(-)
 create mode 100644 libavcodec/h264_changesps_bsf.c

-- 
1.9.2.msysgit.0

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