tinyzimmer / go-gst

Gstreamer bindings and utilities for golang
GNU Lesser General Public License v2.1
130 stars 37 forks source link

panic on GetStructure().UnmarshalInto() #24

Open brucekim opened 2 years ago

brucekim commented 2 years ago

It encounters panic when accessing unexported fields of struct at reflect used by GetStructure().UnmarshallInto().

In my case, I need to get GstRTPPacketLost event. So I need to define type struct in go area in order to extract GstStructure values from gst into go area.

type GstRTPPacketLostEvent struct {
    Seqnum      uint
    Timestamp   uint64
    Duration    uint64
    Retry       uint
}

...ommitted
                var packetLostEvent GstRTPPacketLostEvent
        if err := ev.GetStructure().UnmarshalInto(&packetLostEvent); err != nil {
            logger.Tracef("[src %s] pktlost_probe / cloud not parse RTPPacketLostEvent", s.ID())
            return gst.PadProbePass
        }
...

However it could not get value of each field from GstStructure of GstRTPPacketLost because the field of GstRTPPacketLost is in lowercase whereas the type struct what I defined in go area is starting with upper case for exporting out of struct. I mean, it has not found any matching field in GstRTPPacketLost GstStructure from the type struct I defined above.

https://github.com/GStreamer/gst-plugins-good/blob/20bbeb5e37666c53c254c7b08470ad8a00d97630/gst/rtpmanager/gstrtpjitterbuffer.c#L2433-L2438

    event = gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM,
        gst_structure_new ("GstRTPPacketLost",
            "seqnum", G_TYPE_UINT, (guint) seqnum,
            "timestamp", G_TYPE_UINT64, timestamp,
            "duration", G_TYPE_UINT64, duration,
            "retry", G_TYPE_UINT, num_rtx_retry, NULL));

So I redefined the type struct in go area having filed in lower case only, which results in panic since reflect doesn't allow to set on unexported fields, this causes panic.

type GstRTPPacketLostEvent struct {
    seqnum      uint
    timestamp   uint64
    duration    uint64
    retry       uint
}
panic: reflect: reflect.Value.Set using value obtained using unexported field

goroutine 17 [running, locked to thread]:
reflect.flag.mustBeAssignableSlow(0xc314f2)
        /usr/local/go/src/reflect/value.go:259 +0xc5
reflect.flag.mustBeAssignable(...)
        /usr/local/go/src/reflect/value.go:249
reflect.Value.Set({0xcb4c80, 0xc000412840, 0x6}, {0xcb4c80, 0x157f428, 0xcb4c80})
        /usr/local/go/src/reflect/value.go:1899 +0x6a
github.com/tinyzimmer/go-gst/gst.(*Structure).UnmarshalInto(0xe3980e, {0xc92660, 0xc000412840})
        /home/bkim/git/mpod/vendor/github.com/tinyzimmer/go-gst/gst/gst_structure.go:89 +0x3c6

I figured out an workaround by lowering fieldName then set on UnmarshalInto. However this is limited to cover only lower case of field name.

func (s *Structure) UnmarshalInto(data interface{}) error {
    rv := reflect.ValueOf(data)
    if rv.Kind() != reflect.Ptr || rv.IsNil() {
        return errors.New("Data is invalid (nil or non-pointer)")
    }

    val := reflect.ValueOf(data).Elem()
    nVal := rv.Elem()
    for i := 0; i < val.NumField(); i++ {
        nvField := nVal.Field(i)
        fieldName := val.Type().Field(i).Name
        val, err := s.GetValue(strings.ToLower(fieldName))
        if err == nil {
            nvField.Set(reflect.ValueOf(val))
        }
    }

    return nil
}

I am now figuring out better way for this..

RSWilli commented 1 year ago

@brucekim I have some ideas involving reflection tags to fix this issue. If you still need this, move the issue please and continue the discussion at https://github.com/go-gst/go-gst (where future development of the bindings will take place)