[FFmpeg-devel] [PATCH] FireWire DV/HDV input device using libiec61883

[Stefano Sabatini]

On date Tuesday 2012-05-01 11:43:18 +0200, Georg Lippitsch encoded:
> Here are the two rebased patches, also with the typo-corrections in
> the documentation.
> 
> Georg

> From 72a8b3a412147273f56c9132305af153450c6d62 Mon Sep 17 00:00:00 2001
> From: Georg Lippitsch <georg.lippitsch at gmx.at>
> Date: Mon, 23 Apr 2012 16:01:17 +0200
> Subject: [PATCH 1/2] FireWire DV/HDV input device using libiec61883
> 
> ---
>  configure                |    3 +
>  libavdevice/Makefile     |    1 +
>  libavdevice/alldevices.c |    1 +
>  libavdevice/iec61883.c   |  375 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 380 insertions(+), 0 deletions(-)
>  create mode 100644 libavdevice/iec61883.c
[...]
> +static int iec61883_read_header(AVFormatContext *context)
> +{
> +    struct iec61883_data *dv = context->priv_data;
> +    struct raw1394_portinfo pinf[16];
> +    rom1394_directory rom_dir;
> +    char *endptr;
> +    int inport;
> +    int ports;
> +    int port = -1;
> +    int response;
> +    int i, j = 0;
> +
> +    dv->input_port = -1;
> +    dv->output_port = -1;
> +    dv->channel = -1;
> +
> +    dv->handle = raw1394_new_handle();
> +
> +    if (!dv->handle) {
> +        av_log(context, AV_LOG_ERROR, "Failed to open IEEE1394 interface.\n");
> +        return AVERROR(EIO);
> +    }
> +
> +    if ((ports = raw1394_get_port_info(dv->handle, pinf, 16)) < 0) {
> +        av_log(context, AV_LOG_ERROR, "Failed to get number of IEEE1394 ports.\n");
> +        goto fail;
> +    }
> +

> +    inport = strtol(context->filename, &endptr, 10);
> +    if (endptr != context->filename && *endptr == '\0') {
> +        av_log(context, AV_LOG_INFO, "Selecting IEEE1394 port: %d\n", inport);
> +        j = inport;
> +        ports = inport + 1;
> +    }
> +

What about to explicitely fail in case the string can't be parsed? I
mean you could define a constant like "auto" and abort if the string
can't be parsed as a number. This should prevent bogus input like
"1f" or "1 ".

> +    /* Select first AV/C tape recorder player node */
> +
> +    for (; j < ports && port==-1; ++j) {
> +        if (raw1394_set_port(dv->handle, j)) {
> +            av_log(context, AV_LOG_ERROR, "Failed setting IEEE1394 port.\n");
> +            goto fail;
> +        }
> +        for (i=0; i<raw1394_get_nodecount(dv->handle); ++i) {
> +            if (rom1394_get_directory(dv->handle, i, &rom_dir) < 0)
> +                continue;
> +            if (((rom1394_get_node_type(&rom_dir) == ROM1394_NODE_TYPE_AVC) &&
> +                 avc1394_check_subunit_type(dv->handle, i, AVC1394_SUBUNIT_TYPE_VCR)) ||
> +                (rom_dir.unit_spec_id == MOTDCT_SPEC_ID)) {
> +                rom1394_free_directory(&rom_dir);
> +                dv->node = i;
> +                port = j;
> +                break;
> +            }
> +            rom1394_free_directory(&rom_dir);
> +        }
> +    }
> +
> +    if (port == -1) {
> +        av_log(context, AV_LOG_ERROR, "No AV/C devices found.\n");
> +        goto fail;
> +    }
> +
> +    /* Find out if device is DV or HDV */
> +

> +    if (!dv->is_hdv) {
> +        response = avc1394_transaction(dv->handle, dv->node,
> +                                       AVC1394_CTYPE_STATUS |
> +                                       AVC1394_SUBUNIT_TYPE_TAPE_RECORDER |
> +                                       AVC1394_SUBUNIT_ID_0 |
> +                                       AVC1394_VCR_COMMAND_OUTPUT_SIGNAL_MODE |
> +                                       0xFF, 2);
> +        response = AVC1394_GET_OPERAND0(response);
> +        dv->is_hdv = (response == 0x10 || response == 0x90 || response == 0x1A || response == 0x9A);
> +    } else
> +        dv->is_hdv = dv->is_hdv == IEC61883_HDV ? 1 : 0;

Again I find this quite confusing, since the same field is used for
two different purposes. What I suggest:

    if (dv->type == AUTO) {
        response = avc1394_transaction(dv->handle, dv->node,
                                       AVC1394_CTYPE_STATUS |
                                       AVC1394_SUBUNIT_TYPE_TAPE_RECORDER |
                                       AVC1394_SUBUNIT_ID_0 |
                                       AVC1394_VCR_COMMAND_OUTPUT_SIGNAL_MODE |
                                       0xFF, 2);
        response = AVC1394_GET_OPERAND0(response);
        dv->type = (response == 0x10 || response == 0x90 || response == 0x1A || response == 0x9A) ? HDV : DV;
    }

[...]

> From def5037f0214c1fbb726295574389709bb145469 Mon Sep 17 00:00:00 2001
> From: Georg Lippitsch <georg.lippitsch at gmx.at>
> Date: Wed, 25 Apr 2012 23:08:09 +0200
> Subject: [PATCH 2/2] iec61883 input device documentation
> 
> ---
>  doc/indevs.texi |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 48 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index e699e11..a59f554 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -179,6 +179,54 @@ ffmpeg -f fbdev -frames:v 1 -r 1 -i /dev/fb0 screenshot.jpeg
>  
>  See also @url{http://linux-fbdev.sourceforge.net/}, and fbset(1).
>  
> + at section iec61883
> +
> +FireWire DV/HDV input device using libiec61883.
> +
> +The iec61883 capture device supports capturing from a video device
> +connected via IEEE1394 (FireWire), using libiec61883 and the new Linux
> +FireWire stack (juju). This is the default DV/HDV input method in Linux
> +Kernel 2.6.37 and later, since the old FireWire stack was removed.
> +
> +Specify the FireWire port to be used as input file, or use any non-numeric
> +value (like "auto") to choose the first port connected.
> +
> + at subsection Options
> +
> + at table @option
> +
> + at item dvtype
> +Override autodetection of DV/HDV. This should only be used if auto
> +detection does not work, or if usage of a different device type

> +should be prohibeted. Treating a DV device as HDV (or vice versa) will

typo: prohibited

[...]

Note: you can squeeze the two patches into one.
-- 
FFmpeg = Furious and Foolish Mind-dumbing Pacific Extreme Gem