[FFmpeg-devel] [PATCH 3/7] CrystalHD: Handle different h.264 MBAFF packing.

[Michael Niedermayer]

On Sat, Mar 26, 2011 at 10:50:45AM -0700, Philip Langdale wrote:
> I found another MBAFF sample where the input:output pattern is
> the same as mpeg2 and vc1 (fieldpair input, individual field output).
> While I'm not sure how you can output individual fields from MBAFF,
> if I apply the mpeg2/vc1 handling to this file, it plays correctly.
> 
> So, this changes the detection algorithm to handle the known cases.
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> ---
>  libavcodec/crystalhd.c |   79 +++++++++++++++++++++++++----------------------
>  1 files changed, 42 insertions(+), 37 deletions(-)
> 
> diff --git a/libavcodec/crystalhd.c b/libavcodec/crystalhd.c
> index 3f66b96..95fe931 100644
> --- a/libavcodec/crystalhd.c
> +++ b/libavcodec/crystalhd.c
> @@ -102,10 +102,11 @@
>   ****************************************************************************/
>  
>  typedef enum {
> -    RET_ERROR          = -1,
> -    RET_OK             = 0,
> -    RET_COPY_AGAIN     = 1,
> -    RET_SKIP_NEXT_COPY = 2,
> +    RET_ERROR           = -1,
> +    RET_OK              = 0,
> +    RET_COPY_AGAIN      = 1,
> +    RET_SKIP_NEXT_COPY  = 2,
> +    RET_COPY_NEXT_FIELD = 3,
>  } CopyRet;
>  
>  typedef struct OpaqueList {
> @@ -624,7 +625,13 @@ static inline CopyRet copy_frame(AVCodecContext *avctx,
>          return RET_SKIP_NEXT_COPY;
>      }
>  
> -    return RET_OK;
> +    /*
> +     * Testing has shown that in all cases where we don't want to return the
> +     * full frame immediately, VDEC_FLAG_UNKNOWN_SRC is set.
> +     */
> +    return priv->need_second_field &&
> +           !(output->PicInfo.flags & VDEC_FLAG_UNKNOWN_SRC) ?
> +           RET_COPY_NEXT_FIELD : RET_OK;
>  }
>  
>  
> @@ -806,40 +813,38 @@ static int decode(AVCodecContext *avctx, void *data, int *data_size, AVPacket *a
>  
>      do {
>          rec_ret = receive_frame(avctx, data, data_size, 0);
> -        if (rec_ret == 0 && *data_size == 0) {
> -            if (avctx->codec->id == CODEC_ID_H264) {
> -                /*
> -                 * This case is for when the encoded fields are stored
> -                 * separately and we get a separate avpkt for each one. To keep
> -                 * the pipeline stable, we should return nothing and wait for
> -                 * the next time round to grab the second field.
> -                 * H.264 PAFF is an example of this.
> -                 */
> -                av_log(avctx, AV_LOG_VERBOSE, "Returning after first field.\n");
> -                avctx->has_b_frames--;
> -            } else {
> -                /*
> -                 * This case is for when the encoded fields are stored in a
> -                 * single avpkt but the hardware returns then separately. Unless
> -                 * we grab the second field before returning, we'll slip another
> -                 * frame in the pipeline and if that happens a lot, we're sunk.
> -                 * So we have to get that second field now.
> -                 * Interlaced mpeg2 and vc1 are examples of this.
> -                 */
> -                av_log(avctx, AV_LOG_VERBOSE, "Trying to get second field.\n");
> -                while (1) {
> -                    usleep(priv->decode_wait);
> -                    ret = DtsGetDriverStatus(dev, &decoder_status);
> -                    if (ret == BC_STS_SUCCESS &&
> -                        decoder_status.ReadyListCount > 0) {
> -                        rec_ret = receive_frame(avctx, data, data_size, 1);
> -                        if ((rec_ret == 0 && *data_size > 0) ||
> -                            rec_ret == RET_ERROR)
> -                            break;
> -                    }
> +        if (rec_ret == RET_OK && *data_size == 0) {
> +            /*
> +             * This case is for when the encoded fields are stored
> +             * separately and we get a separate avpkt for each one. To keep
> +             * the pipeline stable, we should return nothing and wait for
> +             * the next time round to grab the second field.
> +             * H.264 PAFF is an example of this.
> +             */
> +            av_log(avctx, AV_LOG_VERBOSE, "Returning after first field.\n");
> +            avctx->has_b_frames--;
> +        } else if (rec_ret == RET_COPY_NEXT_FIELD) {
> +            /*
> +             * This case is for when the encoded fields are stored in a
> +             * single avpkt but the hardware returns then separately. Unless
> +             * we grab the second field before returning, we'll slip another
> +             * frame in the pipeline and if that happens a lot, we're sunk.
> +             * So we have to get that second field now.
> +             * Interlaced mpeg2 and vc1 are examples of this.
> +             */
> +            av_log(avctx, AV_LOG_VERBOSE, "Trying to get second field.\n");
> +            while (1) {
> +                usleep(priv->decode_wait);
> +                ret = DtsGetDriverStatus(dev, &decoder_status);
> +                if (ret == BC_STS_SUCCESS &&
> +                    decoder_status.ReadyListCount > 0) {
> +                    rec_ret = receive_frame(avctx, data, data_size, 1);
> +                    if ((rec_ret == RET_OK && *data_size > 0) ||
> +                        rec_ret == RET_ERROR)
> +                        break;
>                  }

please dont mix reindentions with functional changes, this makes it
hard to review in diffs. Its less a issue in a git tree

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110327/19aa615d/attachment.asc>