Re: [PATCH v2 7/8] winegstreamer: Introduce new wg_transform_read_data entry point.

[Zebediah Figura (she/her)]

On 2/23/22 08:46, Rémi Bernon wrote:
> +static void release_sample_entry(struct wg_sample *sample, struct sample_entry *entry)
> +{
> - GstBuffer *buffer = gst_sample_get_buffer(entry->sample);
> -
> - if (entry->buffer_size > sample->size)
> - {
> -
>    gst_buffer_resize(buffer, sample->size, -1);
> -
>    entry->buffer_size -= sample->size;
> - }
> - else
> - {
> -
>    gst_sample_unref(entry->sample);
> -
>    list_remove(&entry->entry);
> -
>    free(entry);
> - }
>
> +}
> -
>
> +static NTSTATUS copy_from_sample_entry(struct wg_sample *sample, struct sample_entry *entry)
> +{
> - GstBuffer *buffer = gst_sample_get_buffer(entry->sample);
> - GstMapInfo info;
> -
> - if (!gst_buffer_map(buffer, &info, GST_MAP_READ))
> - {
> -
>    GST_ERROR("Failed to map buffer %p", buffer);
> -
>    return STATUS_UNSUCCESSFUL;
> - }
> -
> - if (sample->max_size >= info.size)
> -
>    sample->size = info.size;
> - else
> - {
> -
>    sample->flags |= WG_SAMPLE_FLAG_INCOMPLETE;
> -
>    sample->size = sample->max_size;
> - }
> -
> - memcpy(sample->data, info.data, sample->size);
> - gst_buffer_unmap(buffer, &info);
> -
> - GST_INFO("Copied %u bytes, flags %#x", sample->size, (UINT32)sample->flags);

Isn't that cast redundant?
> - return STATUS_SUCCESS;
>
> +}

release_sample_entry() feels initially confusing to me, or at least more
than necessary. Could we just combine these two functions?

(As far as I can see the only point is to release the buffer it mapping
it fails, but I'd rather just copy the relevant two or three lines.)