From 2dded0fceb11d53d4ca9ececf206025c30b186d5 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Mon, 25 Jan 2010 19:06:53 -0300 Subject: [PATCH] rtpasfpay: Fix packet length semantics Following the ed4d08189ea6e19a50e029e60da52d3583c39fbb commit, this one fixes rtpasfpay to use packet length as the payloaded data length, but also accepting it as the full packet size for compatibility with other implementations due to the lack of clarity of the spec in this part. --- gst/asfmux/gstasfobjects.c | 36 ++++++++++++++++++++++++++++++------ gst/asfmux/gstasfobjects.h | 2 +- gst/asfmux/gstasfparse.c | 3 ++- gst/asfmux/gstrtpasfpay.c | 10 +++++++--- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/gst/asfmux/gstasfobjects.c b/gst/asfmux/gstasfobjects.c index ccb08367b5..a203fc7b4c 100644 --- a/gst/asfmux/gstasfobjects.c +++ b/gst/asfmux/gstasfobjects.c @@ -519,7 +519,7 @@ gst_asf_parse_single_payload (GstByteReader * reader, gboolean * has_keyframe) gboolean gst_asf_parse_packet (GstBuffer * buffer, GstAsfPacketInfo * packet, - gboolean trust_delta_flag) + gboolean trust_delta_flag, guint packet_size) { GstByteReader *reader; gboolean ret = TRUE; @@ -539,6 +539,11 @@ gst_asf_parse_packet (GstBuffer * buffer, GstAsfPacketInfo * packet, guint16 duration = 0; gboolean has_keyframe; + if (packet_size != 0 && GST_BUFFER_SIZE (buffer) != packet_size) { + GST_WARNING ("ASF packets should be aligned with buffers"); + return FALSE; + } + reader = gst_byte_reader_new_from_buffer (buffer); GST_LOG ("Starting packet parsing, size: %u", GST_BUFFER_SIZE (buffer)); @@ -599,11 +604,30 @@ gst_asf_parse_packet (GstBuffer * buffer, GstAsfPacketInfo * packet, padding_len_type, &padd_len)) goto error; - if (packet_len_type != ASF_FIELD_TYPE_NONE && - packet_len != GST_BUFFER_SIZE (buffer)) { - GST_WARNING ("ASF packets should be aligned with buffers"); - ret = FALSE; - goto end; + /* some packet size validation */ + if (packet_size != 0 && packet_len_type != ASF_FIELD_TYPE_NONE) { + if (padding_len_type != ASF_FIELD_TYPE_NONE && + packet_len + padd_len != packet_size) { + GST_WARNING ("Packet size (payload=%u + padding=%u) doesn't " + "match expected size %u", packet_len, padd_len, packet_size); + ret = FALSE; + } + + /* Be forgiving if packet_len has the full packet size + * as the spec isn't really clear on its meaning. + * + * I had been taking it as the full packet size (fixed) + * until bug #607555, that convinced me that it is more likely + * the actual payloaded data size. + */ + if (packet_len == packet_size) { + GST_DEBUG ("This packet's length field represents the full " + "packet and not the payloaded data length"); + ret = TRUE; + } + + if (!ret) + goto end; } GST_LOG ("Getting send time and duration"); diff --git a/gst/asfmux/gstasfobjects.h b/gst/asfmux/gstasfobjects.h index affde86b62..d48a2e17e5 100644 --- a/gst/asfmux/gstasfobjects.h +++ b/gst/asfmux/gstasfobjects.h @@ -109,7 +109,7 @@ guint16 gst_asf_put_subpayload (guint8 * buf, AsfPayload * payload, guint16 size); gboolean gst_asf_parse_packet (GstBuffer * buffer, GstAsfPacketInfo * packet, - gboolean trust_delta_flag); + gboolean trust_delta_flag, guint packet_size); guint64 gst_asf_match_and_peek_obj_size (const guint8 * data, const Guid * guid); gboolean gst_asf_parse_headers (GstBuffer * buffer, GstAsfFileInfo * file_info); diff --git a/gst/asfmux/gstasfparse.c b/gst/asfmux/gstasfparse.c index c05da426cd..8e76a3ccc2 100644 --- a/gst/asfmux/gstasfparse.c +++ b/gst/asfmux/gstasfparse.c @@ -142,7 +142,8 @@ gst_asf_parse_parse_packet (GstAsfParse * asfparse, GstBuffer * buffer) { GstAsfPacketInfo *packetinfo = asfparse->packetinfo; - if (!gst_asf_parse_packet (buffer, packetinfo, FALSE)) + if (!gst_asf_parse_packet (buffer, packetinfo, FALSE, + asfparse->asfinfo->packet_size)) goto error; GST_DEBUG_OBJECT (asfparse, "Received packet of length %" G_GUINT32_FORMAT diff --git a/gst/asfmux/gstrtpasfpay.c b/gst/asfmux/gstrtpasfpay.c index ffcfd85ab3..302cf3e230 100644 --- a/gst/asfmux/gstrtpasfpay.c +++ b/gst/asfmux/gstrtpasfpay.c @@ -141,7 +141,8 @@ gst_rtp_asf_pay_handle_packet (GstRtpAsfPay * rtpasfpay, GstBuffer * buffer) rtppay = GST_BASE_RTP_PAYLOAD (rtpasfpay); packetinfo = &rtpasfpay->packetinfo; - if (!gst_asf_parse_packet (buffer, packetinfo, TRUE)) { + if (!gst_asf_parse_packet (buffer, packetinfo, TRUE, + rtpasfpay->asfinfo.packet_size)) { GST_ERROR_OBJECT (rtpasfpay, "Error while parsing asf packet"); gst_buffer_unref (buffer); return GST_FLOW_ERROR; @@ -176,10 +177,13 @@ gst_rtp_asf_pay_handle_packet (GstRtpAsfPay * rtpasfpay, GstBuffer * buffer) default: break; } - gst_asf_parse_packet (buffer, &info, FALSE); + gst_asf_parse_packet (buffer, &info, FALSE, 0); } - packet_util_size = packetinfo->packet_size - packetinfo->padding; + if (packetinfo->padding != 0) + packet_util_size = rtpasfpay->asfinfo.packet_size - packetinfo->padding; + else + packet_util_size = packetinfo->packet_size; packet_offset = 0; while (packet_util_size > 0) { /* Even if we don't fill completely an output buffer we