check_next is evaluated after request resolution

[vitoyucepi]

Describe the bug
The playlist.list function describes the check_next parameter as follows

The function is called before resolution

liquidsoap/src/libs/playlist.liq

Lines 175 to 180 in e536a57

	# @param ~check_next Function used to filter next tracks. A candidate track is \
	# only validated if the function returns true on it. The function is called \
	# before resolution, hence metadata will only be available for requests \
	# corresponding to local files. This is typically used to avoid repetitions, \
	# but be careful: if the function rejects all attempts, the playlist will \
	# enter into a consuming loop and stop playing anything.

But in reality, it's done after resolution.

liquidsoap/src/libs/playlist.liq

Lines 361 to 410 in e536a57

	r =
	request.create(
	cue_in_metadata=cue_in_metadata,
	cue_out_metadata=cue_out_metadata,
	file
	)
	if
	not request.resolve(content_type=s, r)
	then
	log.info(
	label=id,
	"Could not resolve request: #{request.uri(r)}."
	)
	request.destroy(r)
	ref.incr(failed_count)
	# Playlist failed, call handler.
	if
	failed_count() < max_fail
	then
	log.info(
	label=id,
	"Playlist failed."
	)
	if
	null.defined(on_fail)
	then
	f = null.get(on_fail)
	reload(f())
	end
	end
	failed_time := time()
	(next() : request?)
	else
	failed_count := 0
	if
	check_next(r)
	then
	r
	else
	log.info(
	label=id,
	"Request #{request.uri(r)} rejected by check_next."
	)
	request.destroy(r)
	next()
	end
	end

To Reproduce

1. Generate music and pictures

   mkdir music;
   convert -size 600x600 plasma: music/cover.png;
   ffmpeg -f lavfi -i "sine=frequency=100:duration=1" music/1.mp3;

2. compose.yaml

   version: "3.8"
   services:
     liquidsoap:
       image: savonet/liquidsoap:v2.2.3
       command:
         - "/tmp/test/main.liq"
       volumes:
         - ./:/tmp/test

3. main.liq

   settings.log.level := 1
   
   def check_next(r)
     log.critical("#{request.uri(r)}")
     false
   end
   
   s = mksafe(playlist("/tmp/test/music", mode="normal", check_next=check_next))
   s = clock(sync="none", s)
   output.dummy(s)
   

Expected behavior
I'd like to use check_next to prevent resolution of image or garbage files before they are decoded.

Version details

• OS: Debian in docker container
• Version: 2.2.3, e536a57

Install method
savonet/liquidsoap:v2.2.3

Common issues
N/A

[toots]

Thanks for sumbitting. I have changed the order of operations. This is too big of a change to go into a bugfix release, however.

[vitoyucepi]

Hi @toots,
What will happen if the playlist contains remote files and check_next uses file.mime?

[toots]

@vitoyucepi do you have a specific example in mind?

[vitoyucepi]

settings.log.level := 3

def is_audio(_request)
  name = request.uri(_request)
  mime = file.mime(name)
  if mime == null() then
    false
  else
    mime = null.get(mime)
    if string.contains(mime, prefix="audio") then
      true
    else
      false
    end
  end
end

s = mksafe(playlist("/tmp/test/playlist.m3u", mode="normal", check_next=is_audio))
s = clock(sync="none", s)
output.dummy(s)

I'd like to check if the file is audio/*, but playlist.m3u has http:// links in it.
I think the remote file will be downloaded during request.resolve, so file.mime will throw an error:

Failed to obtain a media request: Magic.Failure("Magic.file: cannot stat `http://www.example.com/~user/Mine.mp3' (No such file or directory)")