[libav-devel] [PATCH] cbs_h264: Fix writing streams with auxiliary pictures

[Mark Thompson]

Tested with the alphaconformanceG test sample.
---
On 22/09/17 15:24, James Almer wrote:
> On 9/21/2017 8:51 AM, Vittorio Giovara wrote:
>> ---
>> This is to silence this warning (which, admittedly, could also be ignored)
>>
>> cbs_h2645.c:1007:58: warning: variable 'sps_ext' is uninitialized when used 
>> here [-Wuninitialized]
>> err = cbs_h264_write_sps_extension(ctx, pbc, sps_ext);
>> ^~~
>> cbs_h2645.c:1005:41: note: initialize the variable 'sps_ext' to silence this 
>> warning
>> H264RawSPSExtension *sps_ext;
>> ^
>> = NULL
>>
>>  libavcodec/cbs_h2645.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index 50a227da78..6fe138188a 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -1002,7 +1002,7 @@ static int 
>> cbs_h264_write_nal_unit(CodedBitstreamContext *ctx,
>>  
>>  case H264_NAL_SPS_EXT:
>>  {
>> -H264RawSPSExtension *sps_ext;
>> +H264RawSPSExtension *sps_ext = NULL;
> 
> Judging by how other NAL units are handled in this function, I think
> this should be
> 
> H264RawSPSExtension *sps_ext = unit->content;
> 
> Guess no fate test has a sample with an SPS ext unit?

Yes, it needs the alphaconformanceG file which is not in the fate suite.  (I 
guess I only tested trace_headers, not h264_metdata.)

>>  
>>  err = cbs_h264_write_sps_extension(ctx, pbc, sps_ext);
>>  if (err < 0)
>>

 libavcodec/cbs_h2645.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 50a227da7..a1b887fd4 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -1002,7 +1002,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext 
*ctx,
 
 case H264_NAL_SPS_EXT:
 {
-H264RawSPSExtension *sps_ext;
+H264RawSPSExtension *sps_ext = unit->content;
 
 err = cbs_h264_write_sps_extension(ctx, pbc, sps_ext);
 if (err < 0)
@@ -1026,6 +1026,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext 
*ctx,
 
 case H264_NAL_SLICE:
 case H264_NAL_IDR_SLICE:
+case H264_NAL_AUXILIARY_SLICE:
 {
 H264RawSlice *slice = unit->content;
 BitstreamContext bc;
-- 
2.14.1
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] [RFC] lavc: external hardware frame pool initialization, take 1246

[wm4]

From: wm4 

I feel like this has been beaten to death, but let's do it one more
time.

No code, this is just about the API itself.

I also found a way to make one of Mark T.'s RFCs do what I want, so
another proposal is included in the doxygen comments.
---
 libavcodec/avcodec.h | 96 
 1 file changed, 96 insertions(+)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 162f1abe4b..5e7735ae1a 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3436,6 +3436,102 @@ int avcodec_open2(AVCodecContext *avctx, const AVCodec 
*codec, AVDictionary **op
  */
 int avcodec_close(AVCodecContext *avctx);
 
+/**
+ * Fill the given hw_frames struct with values adequate for hardware decoding.
+ * This is meant to get called from the get_format callback. There are a number
+ * of requirements for calling this function:
+ *
+ * - It must be called from get_format with the same avctx parameter if you
+ *   expect it to fill correct parameters. Calling it outside of get_format may
+ *   or may not work correctly, but only within get_format, enough of the video
+ *   bitstream will be parsed. This API function may access arbitrary
+ *   AVCodecContext fields (even non-public ones), so the API user cannot know
+ *   or ensure whether this will work correctly outside of get_format.
+ * - The hw_pix_fmt must be one of the choices suggested by get_format, and if
+ *   the user decides to use a hw_frames context prepared with this API 
function,
+ *   the user must return the same hw_pix_fmt from get_format. hw_pix_fmt
+ *   selects the hwaccel implementation, and most parameters are implementation
+ *   dependent.
+ * - The hw_frames context must be allocated from a device type that supports
+ *   the given hw_pix_fmt.
+ * - The hw_frames context must not have been initialized yet. The API function
+ *   may overwrite any fields in the hw_frames struct. A user can modify the
+ *   parameters after calling the function. Once that is done, the hw_frames
+ *   context must be initialized. Then set AVCodecContext.hw_frames_ctx to it.
+ *   All of this has to happen before returning from get_format.
+ * - It is perfectly possible to call this function, without actually using
+ *   the resulting hw_frames context. A common use-case might be reusing a
+ *   previously initialized hw_frames_ctx, and only calling this function on
+ *   a dummy hw_frames_ctx to check whether the codec requirements have 
changed.
+ *
+ * The function will set at least the following fields on hw_frames 
(potentially
+ * more, depending on hwaccel API):
+ *
+ * - Set the format field to hw_pix_fmt.
+ * - Set the sw_format field to the most suited and most versatile format. (An
+ *   implication is that this will require generic formats over opaque formats
+ *   with arbitrary restrictions, if possible.)
+ * - Set the width/height fields to the coded frame size, rounded up to the
+ *   minimum alignment.
+ * - Only _if_ the hwaccel requires a pre-allocated pool: set the 
initial_pool_size
+ *   field to the number of maximum reference surfaces possible with the codec,
+ *   plus 1 surface for the user to work (meaning the user can safely reference
+ *   at most 1 decoded surface at a time). If the hwaccel does not require pre-
+ *   allocation, the field is left to 0, and the codec will allocate new
+ *   surfaces on demand during decoding.
+ *
+ * This API is for decoding with hardware acceleration only. Calling this
+ * function is not a requirement. An API user can setup
+ * AVCodecContext.hw_frames_ctx manually, or set AVCodecContext.hw_device_ctx
+ * (which will make the hwaccel implementation setup hw_frames_ctx fully
+ * automatically).
+ *
+ *  To discuss: 
+ *
+ * - Does this require the implementation to lookup the codec profile twice in
+ *   the worst case? (See vaapi, or if d3d11va checked whether it has to use
+ *   the opaque surface format on very restricted devices.)
+ * - Should there be a way to select difference performance or convenience
+ *   characteristics? For example, there might be cases where selecting an 
opaque
+ *   pixel format could be faster than nv12, or such. Or use cases like 
enabling
+ *   shared surfaces in an API-independent way. The current API wouldn't allow
+ *   passing such hints (except AV_HWACCEL_FLAG_* or new avctx fields).
+ * - Should there be a way to enable preallocation always?
+ * - Having to call this in get_format is unavoidable, but the fragile use 
rules
+ *   are still annoying. The earlier proposed "refine" callback would at least
+ *   have to advantage that the user could not call it at the "wrong" time.
+ *   Possible modification of the earlier proposal to make it roughly as 
powerful
+ *   as this proposed API:
+ *
+ *  struct AVCodecContext {
+ *  ...
+ *
+ *  / **
+ *   * Called when hwaccel hardware decoding is setup. 
avctx->hw_frames_ctx
+ *   * will 

Re: [libav-devel] [PATCH] configure: cosmetics: Wrap a configure error string to the paragraph length

[Vittorio Giovara]

On Sat, Sep 23, 2017 at 11:14 AM, Diego Biurrun  wrote:

> On Thu, Sep 21, 2017 at 07:08:00PM +0200, Vittorio Giovara wrote:
> > On Thu, Sep 21, 2017 at 6:34 PM, Diego Biurrun  wrote:
> > > On Thu, Sep 21, 2017 at 01:37:03PM +0200, Vittorio Giovara wrote:
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -401,8 +401,8 @@ include the log this produces with your report.
> > > >  EOF
> > > >  else
> > > >  cat < > > > -Include the log file "$logfile" produced by configure as this will
> help
> > > > -solving the problem.
> > > > +Include the log file "$logfile" produced by configure as this
> > > > +will help solving the problem.
> > > >  EOF
> > >
> > > What paragraph length are you wrapping this to? It currently matches
> the
> > > length of the previous output lines quite well ...
> >
> > I'm talking about the output lines on a 80chars console
> > This is how I see it: https://www.dropbox.com/s/
> qfpgk7i9qs3htwk/Screenshot%
> > 202017-09-21%2019.05.15.png?dl=0
>
> OK, noted. I'll queue it up with more cosmetics that might be coming, OK?
>

ok with me
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] configure: cosmetics: Wrap a configure error string to the paragraph length

[Diego Biurrun]

On Thu, Sep 21, 2017 at 07:08:00PM +0200, Vittorio Giovara wrote:
> On Thu, Sep 21, 2017 at 6:34 PM, Diego Biurrun  wrote:
> > On Thu, Sep 21, 2017 at 01:37:03PM +0200, Vittorio Giovara wrote:
> > > --- a/configure
> > > +++ b/configure
> > > @@ -401,8 +401,8 @@ include the log this produces with your report.
> > >  EOF
> > >  else
> > >  cat < > > -Include the log file "$logfile" produced by configure as this will help
> > > -solving the problem.
> > > +Include the log file "$logfile" produced by configure as this
> > > +will help solving the problem.
> > >  EOF
> >
> > What paragraph length are you wrapping this to? It currently matches the
> > length of the previous output lines quite well ...
> 
> I'm talking about the output lines on a 80chars console
> This is how I see it: https://www.dropbox.com/s/qfpgk7i9qs3htwk/Screenshot%
> 202017-09-21%2019.05.15.png?dl=0

OK, noted. I'll queue it up with more cosmetics that might be coming, OK?

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel