Skip to content

Commit

Permalink
Introduce Tprintf to simplify the current log format
Browse files Browse the repository at this point in the history
Hypcast previously had a bunch of individual "logf" methods to dump out
a log prefixed by a type name and pointer address. This change uses a
bit of reflection and a bit of rework on the existing type names to
extract that into one shared function.

There's an interesting mix of generics and reflection here. I'm using
the type parameter to enforce that src is a pointer, which guarantees
that "%p" will work right and simplifies getting the type name compared
to digging through layers of Elem() that will panic with a wrong move.

I'm considering moving Hypcast to log/slog. Even if I throw this away
soon, it's nice that I'll be able to search for "log." across the whole
codebase to see what needs to change.
  • Loading branch information
ahamlinman committed Sep 7, 2024
1 parent 3eb419d commit 3b669fd
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 47 deletions.
30 changes: 11 additions & 19 deletions internal/api/tunerstatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ package api

import (
"context"
"log"
"net/http"
"sync"

"github.com/gorilla/websocket"

"github.com/ahamlinman/hypcast/internal/atsc/tuner"
"github.com/ahamlinman/hypcast/internal/log"
"github.com/ahamlinman/hypcast/internal/watch"
)

type tunerStatusHandler struct {
type TunerStatusHandler struct {
tuner *tuner.Tuner
socket *websocket.Conn
watch watch.Watch
Expand All @@ -23,19 +23,19 @@ type tunerStatusHandler struct {

func (h *Handler) handleSocketTunerStatus(w http.ResponseWriter, r *http.Request) {
ctx, shutdown := context.WithCancelCause(r.Context())
tsh := &tunerStatusHandler{
tsh := &TunerStatusHandler{
tuner: h.tuner,
ctx: ctx,
shutdown: shutdown,
}
tsh.ServeHTTP(w, r)
}

func (tsh *tunerStatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
tsh.logf("Starting new connection")
func (tsh *TunerStatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
log.Tprintf(tsh, "Starting new connection")
defer func() {
tsh.waitForCleanup()
tsh.logf("Connection done: %v", context.Cause(tsh.ctx))
log.Tprintf(tsh, "Connection done: %v", context.Cause(tsh.ctx))
}()

var err error
Expand All @@ -57,16 +57,16 @@ func (tsh *tunerStatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
<-tsh.ctx.Done()
}

func (tsh *tunerStatusHandler) sendNewTunerStatus(s tuner.Status) {
tsh.logf("Received tuner status: %v", s)
func (tsh *TunerStatusHandler) sendNewTunerStatus(s tuner.Status) {
log.Tprintf(tsh, "Received tuner status: %v", s)

msg := tsh.mapTunerStatusToMessage(s)
if err := tsh.socket.WriteJSON(msg); err != nil {
tsh.shutdown(err)
}
}

func (tsh *tunerStatusHandler) drainClient() {
func (tsh *TunerStatusHandler) drainClient() {
// Per https://pkg.go.dev/github.com/gorilla/websocket#hdr-Control_Messages,
// we have to drain incoming messages ourselves even if we don't care about
// them.
Expand All @@ -78,7 +78,7 @@ func (tsh *tunerStatusHandler) drainClient() {
}
}

func (tsh *tunerStatusHandler) waitForCleanup() {
func (tsh *TunerStatusHandler) waitForCleanup() {
if tsh.watch != nil {
tsh.watch.Wait()
}
Expand All @@ -97,7 +97,7 @@ var tunerStateStrings = map[tuner.State]string{
tuner.StatePlaying: "Playing",
}

func (tsh *tunerStatusHandler) mapTunerStatusToMessage(s tuner.Status) tunerStatusMsg {
func (tsh *TunerStatusHandler) mapTunerStatusToMessage(s tuner.Status) tunerStatusMsg {
msg := tunerStatusMsg{
State: tunerStateStrings[s.State],
ChannelName: s.ChannelName,
Expand All @@ -107,11 +107,3 @@ func (tsh *tunerStatusHandler) mapTunerStatusToMessage(s tuner.Status) tunerStat
}
return msg
}

func (tsh *tunerStatusHandler) logf(format string, v ...any) {
joinFmt := "TunerStatusHandler(%p): " + format
joinArgs := make([]any, len(v)+1)
joinArgs[0] = tsh
copy(joinArgs[1:], v)
log.Printf(joinFmt, joinArgs...)
}
42 changes: 17 additions & 25 deletions internal/api/webrtc.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import (
"context"
"encoding/json"
"errors"
"log"
"net/http"
"sync"

"github.com/gorilla/websocket"
"github.com/pion/webrtc/v3"

"github.com/ahamlinman/hypcast/internal/atsc/tuner"
"github.com/ahamlinman/hypcast/internal/log"
"github.com/ahamlinman/hypcast/internal/watch"
)

Expand Down Expand Up @@ -43,7 +43,7 @@ func init() {
webrtcAPI = webrtc.NewAPI(webrtc.WithMediaEngine(&me))
}

type webrtcHandler struct {
type WebRTCHandler struct {
tuner *tuner.Tuner
socket *websocket.Conn
rtcPeer *webrtc.PeerConnection
Expand All @@ -55,19 +55,19 @@ type webrtcHandler struct {

func (h *Handler) handleSocketWebRTCPeer(w http.ResponseWriter, r *http.Request) {
ctx, shutdown := context.WithCancelCause(r.Context())
wh := &webrtcHandler{
wh := &WebRTCHandler{
tuner: h.tuner,
ctx: ctx,
shutdown: shutdown,
}
wh.ServeHTTP(w, r)
}

func (wh *webrtcHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
wh.logf("Starting new connection")
func (wh *WebRTCHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
log.Tprintf(wh, "Starting new connection")
defer func() {
wh.waitForCleanup()
wh.logf("Connection done: %v", context.Cause(wh.ctx))
log.Tprintf(wh, "Connection done: %v", context.Cause(wh.ctx))
}()

var err error
Expand Down Expand Up @@ -95,7 +95,7 @@ func (wh *webrtcHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
<-wh.ctx.Done()
}

func (wh *webrtcHandler) handleClientSessionAnswers() {
func (wh *WebRTCHandler) handleClientSessionAnswers() {
for {
_, r, err := wh.socket.NextReader()
if err != nil {
Expand All @@ -116,8 +116,8 @@ func (wh *webrtcHandler) handleClientSessionAnswers() {
}
}

func (wh *webrtcHandler) handleTrackUpdate(ts tuner.Tracks) {
wh.logf("Received tracks: %v", ts)
func (wh *WebRTCHandler) handleTrackUpdate(ts tuner.Tracks) {
log.Tprintf(wh, "Received tracks: %v", ts)
if err := wh.replaceTracks(ts); err != nil {
wh.shutdown(err)
return
Expand All @@ -128,7 +128,7 @@ func (wh *webrtcHandler) handleTrackUpdate(ts tuner.Tracks) {
}
}

func (wh *webrtcHandler) replaceTracks(ts tuner.Tracks) error {
func (wh *WebRTCHandler) replaceTracks(ts tuner.Tracks) error {
if err := wh.removeTracks(); err != nil {
return err
}
Expand All @@ -138,7 +138,7 @@ func (wh *webrtcHandler) replaceTracks(ts tuner.Tracks) error {
return wh.addTracks(ts)
}

func (wh *webrtcHandler) renegotiateSession() error {
func (wh *WebRTCHandler) renegotiateSession() error {
if !wh.hasTransceivers() {
// Skip negotiation until we've had a chance to properly define video and
// audio transceivers based on Tuner tracks.
Expand All @@ -163,7 +163,7 @@ func (wh *webrtcHandler) renegotiateSession() error {
return wh.socket.WriteJSON(msg)
}

func (wh *webrtcHandler) removeTracks() error {
func (wh *WebRTCHandler) removeTracks() error {
for _, sender := range wh.rtcPeer.GetSenders() {
if err := wh.rtcPeer.RemoveTrack(sender); err != nil {
return err
Expand All @@ -172,14 +172,14 @@ func (wh *webrtcHandler) removeTracks() error {
return nil
}

func (wh *webrtcHandler) addTracks(ts tuner.Tracks) error {
func (wh *WebRTCHandler) addTracks(ts tuner.Tracks) error {
if wh.hasTransceivers() {
return wh.addTracksWithExistingTransceivers(ts)
}
return wh.addTracksWithNewTransceivers(ts)
}

func (wh *webrtcHandler) addTracksWithExistingTransceivers(ts tuner.Tracks) error {
func (wh *WebRTCHandler) addTracksWithExistingTransceivers(ts tuner.Tracks) error {
if _, err := wh.rtcPeer.AddTrack(ts.Video); err != nil {
return err
}
Expand All @@ -189,7 +189,7 @@ func (wh *webrtcHandler) addTracksWithExistingTransceivers(ts tuner.Tracks) erro
return nil
}

func (wh *webrtcHandler) addTracksWithNewTransceivers(ts tuner.Tracks) error {
func (wh *WebRTCHandler) addTracksWithNewTransceivers(ts tuner.Tracks) error {
init := webrtc.RTPTransceiverInit{
Direction: webrtc.RTPTransceiverDirectionSendonly,
}
Expand All @@ -202,21 +202,13 @@ func (wh *webrtcHandler) addTracksWithNewTransceivers(ts tuner.Tracks) error {
return nil
}

func (wh *webrtcHandler) hasTransceivers() bool {
func (wh *WebRTCHandler) hasTransceivers() bool {
return len(wh.rtcPeer.GetTransceivers()) > 0
}

func (wh *webrtcHandler) waitForCleanup() {
func (wh *WebRTCHandler) waitForCleanup() {
if wh.watch != nil {
wh.watch.Wait()
}
wh.waitGroup.Wait()
}

func (wh *webrtcHandler) logf(format string, v ...any) {
joinFmt := "WebRTCHandler(%p): " + format
joinArgs := make([]any, len(v)+1)
joinArgs[0] = wh
copy(joinArgs[1:], v)
log.Printf(joinFmt, joinArgs...)
}
6 changes: 3 additions & 3 deletions internal/atsc/tuner/tuner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package tuner
import (
"errors"
"fmt"
"log"
"strings"
"sync"
"text/template"
Expand All @@ -16,6 +15,7 @@ import (

"github.com/ahamlinman/hypcast/internal/atsc"
"github.com/ahamlinman/hypcast/internal/gst"
"github.com/ahamlinman/hypcast/internal/log"
"github.com/ahamlinman/hypcast/internal/watch"
)

Expand Down Expand Up @@ -194,12 +194,12 @@ func (t *Tuner) Tune(channelName string) (err error) {
t.pipeline.SetSink(sinkNameVideo, createTrackSink(vt))
t.pipeline.SetSink(sinkNameAudio, createTrackSink(at))

log.Printf("Tuner(%p): Starting pipeline", t)
log.Tprintf(t, "Starting pipeline")
err = t.pipeline.Start()
if err != nil {
return err
}
log.Printf("Tuner(%p): Started pipeline", t)
log.Tprintf(t, "Started pipeline")

t.status.Set(Status{
State: StatePlaying,
Expand Down
20 changes: 20 additions & 0 deletions internal/log/log.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Package log provides special log formatting features for Hypcast.
package log

import (
"log"
"reflect"
)

// Tprintf prints its arguments in the manner of [log.Printf], with a prefix of
// the form "Type(0xabcd...)" indicating the element type and address of the src
// pointer. This can be a convenient way to differentiate instances of the same
// type, though addresses are somewhat opaque as identifiers, and revealing them
// might even be a security concern (albeit a far-fetched one).
func Tprintf[T any](src *T, fmt string, v ...any) {
tfmt := "%s(%p): " + fmt
tval := make([]any, len(v)+2)
tval[0], tval[1] = reflect.TypeFor[T]().Name(), src
copy(tval[2:], v)
log.Printf(tfmt, tval...)
}

0 comments on commit 3b669fd

Please sign in to comment.